diff --git a/.github/workflows/python-sdk-publish.yml b/.github/workflows/python-sdk-publish.yml index f185707..bc85f6a 100644 --- a/.github/workflows/python-sdk-publish.yml +++ b/.github/workflows/python-sdk-publish.yml @@ -20,6 +20,9 @@ jobs: build: name: Build distribution runs-on: ubuntu-24.04 + # Each job here usually takes under a minute. The timeouts end a hung + # step long before GitHub's default of six hours. + timeout-minutes: 10 steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -77,7 +80,14 @@ jobs: # published: the wheel, and the sdist, since a wheel built from the sdist # (pip install --no-binary, a distribution's packager) holds only what # the sdist does. - - name: Check the wheel and sdist ship their type information + # + # Nor may either one ship a package other than permit: permit 2.8.3's + # wheel installed a top-level `tests` package, which shadowed the + # consumer's own `tests` module. [tool.uv.build-backend] in + # pyproject.toml keeps it out now; this check fails the release if it + # comes back. test.yml's compatibility job runs the same check on pull + # requests to main; keep the two identical. + - name: Check the wheel and sdist contents run: | set -euo pipefail uv run --no-project python - dist <<'PY' @@ -86,22 +96,39 @@ jobs: import zipfile from pathlib import Path + REQUIRED = ["permit/py.typed", "permit/_sync_types.pyi"] + + + def check(artifact: Path, names: set[str], found: set[str], allowed: set[str]) -> None: + missing = [path for path in REQUIRED if path not in names] + if missing: + sys.exit(f"{artifact.name} is missing {missing}") + unexpected = sorted(found - allowed) + if unexpected: + sys.exit(f"{artifact.name} holds {unexpected}; only {sorted(allowed)} may ship") + print(f"{artifact.name} ships {' and '.join(REQUIRED)} and no package beside permit") + + dist = Path(sys.argv[1]) wheels = sorted(dist.glob("*.whl")) sdists = sorted(dist.glob("*.tar.gz")) if len(wheels) != 1 or len(sdists) != 1: found = [path.name for path in wheels + sdists] sys.exit(f"expected one wheel and one sdist in {dist}, found {found}") - required = ["permit/py.typed", "permit/_sync_types.pyi"] - contents = {wheels[0]: set(zipfile.ZipFile(wheels[0]).namelist())} - # Every sdist path starts with its top-level permit-/ directory. - with tarfile.open(sdists[0]) as sdist: - contents[sdists[0]] = {name.partition("/")[2] for name in sdist.getnames()} - for artifact, names in contents.items(): - missing = [path for path in required if path not in names] - if missing: - sys.exit(f"{artifact.name} is missing {missing}") - print(f"{artifact.name} ships {' and '.join(required)}") + wheel, sdist = wheels[0], sdists[0] + + # A wheel's top level is what lands in site-packages: permit/ and its + # permit-.dist-info, named after permit--.whl. + with zipfile.ZipFile(wheel) as wheel_file: + names = set(wheel_file.namelist()) + dist_info = "-".join(wheel.name.split("-")[:2]) + ".dist-info" + check(wheel, names, {name.split("/")[0] for name in names}, {"permit", dist_info}) + + # Every sdist path starts with its permit-/ directory. Below + # it, the files are metadata and docs, and the only directory is permit/. + with tarfile.open(sdist) as sdist_file: + names = {name.partition("/")[2] for name in sdist_file.getnames()} + check(sdist, names, {name.split("/")[0] for name in names if "/" in name}, {"permit"}) PY - name: Upload distribution @@ -114,6 +141,7 @@ jobs: scan: name: Security Gate runs-on: ubuntu-24.04 + timeout-minutes: 15 needs: [build] steps: - name: Checkout code @@ -150,6 +178,12 @@ jobs: # See the same step in security.yml: this only installs Trivy, and # hide-progress keeps its empty scan from logging a warning. hide-progress: true + # The action's cache is on by default and restores the Trivy binary + # itself from the Actions cache, with no checksum check, so a cache + # entry would decide which scanner the release gate runs. Off: every + # release downloads the binary and checks it against the checksums + # of its Trivy release. + cache: false # The migration skill's sample apps pin vulnerable versions on # purpose and are never installed (skills/tests/README.md). skip-dirs: skills/tests/fixtures @@ -209,15 +243,21 @@ jobs: publish: name: Publish to PyPI runs-on: ubuntu-24.04 + timeout-minutes: 10 needs: [scan] + # PyPI trusted publishing: this job holds no PyPI token. PyPI accepts its + # upload because this repository, this workflow file and this environment + # are registered as a trusted publisher of the permit project on pypi.org. + # Renaming any of the three stops releases until that registration is + # changed to match. PyPI does not check which branch or tag the job ran + # from, so a branch that edits this file to run on push could upload too. + # What limits uploads to release tags is the pypi environment's deployment + # rules, a repository setting, not anything in this file. environment: name: pypi url: https://pypi.org/p/permit permissions: - # id-token is what lets gh-action-pypi-publish attach PEP 740 build - # attestations. contents/pull-requests write were previously granted and - # never used -- nothing in this workflow commits or opens a PR. - id-token: write + id-token: write # OIDC token PyPI exchanges for an upload token; also signs attestations steps: # NODE_OPTIONS: the unzip library download-artifact v8.0.1 bundles still # calls the deprecated Buffer() constructor, so every download prints @@ -231,14 +271,6 @@ jobs: name: dist path: dist/ + # No password: with no token given, the action authenticates by OIDC. - name: Publish package distributions to PyPI uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 - with: - # zizmor: ignore[use-trusted-publishing] - # TODO: migrate to PyPI Trusted Publishing (OIDC) and drop this - # secret. That cannot be done from this repo alone -- it requires - # registering permitio/permit-python + this workflow filename + - # the "pypi" environment as a trusted publisher on PyPI first. - # Flipping the workflow before that is configured would break the - # next release, so it is deliberately left as a follow-up. - password: ${{ secrets.PYPI_TOKEN }} diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index db1e9be..e34e488 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -6,9 +6,9 @@ on: # a required check that never runs as perpetually pending rather than # passing, so a path filter here would block every PR that happens not to # touch a dependency file. The audit takes under two minutes, which is - # cheaper than that failure mode. + # cheaper than that failure mode. Not base-filtered either: a stacked PR, + # whose base is another PR's branch, gets the same checks before it merges. pull_request: - branches: [main, master] # Run on every merge to main too, so a regression is surfaced immediately # (failed run on main) rather than waiting for the next PR to trip over it. # No PR comment is posted on push; the job summary carries the detail. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c4b2b09..45fe666 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,9 +1,8 @@ name: Test on: + # Every PR, whatever its base: a stacked PR, whose base is another PR's + # branch, gets the full suite before it merges. pull_request: - branches: - - main - - master push: branches: - main @@ -17,10 +16,22 @@ permissions: env: PROJECT_ID: 7f55831d77c642739bc17733ab0af138 #github actions project id (under 'Permit.io Tests' workspace) ENV_NAME: python-sdk-ci + # The PDP the required `pytest` jobs run against, pinned by version and by the + # digest of that version's multi-arch image index, so a new PDP release cannot + # fail a required check. Docker pulls by the digest; the tag only names it. + # Dependabot does not update this. The `e2e (latest PDP image)` job runs the + # suite against permitio/pdp-v2:latest, so a new release shows up there first. + # To move the pin, take the version's `digest` from + # https://hub.docker.com/v2/repositories/permitio/pdp-v2/tags/. + PINNED_PDP_IMAGE: >- + permitio/pdp-v2:0.9.16@sha256:e3cf30794ec2d256636b4714641df46e51ee58a3f1f0d24c606e214e0bf8669a jobs: pytest: runs-on: ubuntu-24.04 + # A run takes 5-6 minutes, the PDP wait included. The limit stops a hung + # test from holding a runner for GitHub's default of six hours. + timeout-minutes: 30 strategy: fail-fast: false matrix: @@ -121,13 +132,14 @@ jobs: - name: Start the PDP env: ENV_API_KEY: ${{ env.ENV_API_KEY }} + PDP_IMAGE: ${{ env.PINNED_PDP_IMAGE }} run: | set -euo pipefail docker run -d --name permit-pdp \ -p 7766:7000 \ -e PDP_API_KEY="${ENV_API_KEY}" \ -e PDP_DEBUG=true \ - permitio/pdp-v2:latest + "${PDP_IMAGE}" echo "PDP container started; it warms up while dependencies install." # --locked fails the job if uv.lock is out of date with pyproject.toml @@ -224,11 +236,213 @@ jobs: echo "::warning title=Scratch env leaked::Failed to delete environment ${ENV_ID}. Delete it by hand." fi + # The e2e tests against PDPs this repository does not pin. Neither leg is a + # required check, so a new PDP release or a change to the cloud PDP shows up + # here without blocking a PR. + # - latest PDP image: the suite against permitio/pdp-v2:latest, on pydantic 2. + # Red here with `pytest` green means the newest PDP release behaves unlike + # PINNED_PDP_IMAGE. + # - cloud PDP: tests/test_cloud_pdp_e2e.py against the hosted cloud PDP. Each + # test builds a small RBAC policy in the scratch environment, waits for the + # cloud PDP to apply it, and asserts the exact answers of check, bulk_check, + # get_user_permissions and filter_objects. The module skips wherever PDP_URL + # points at a PDP container (see that module). + # Each leg makes its own scratch environment, keyed by run, attempt and leg, + # so it never shares one with a `pytest` lane, the other leg or a re-run. + e2e-unpinned-pdp: + name: e2e (${{ matrix.pdp }}) + # Starts once both `pytest` lanes pass, so it never repeats a failure they + # already report. With them green, a red latest-PDP leg points at the PDP. + needs: pytest + runs-on: ubuntu-24.04 + timeout-minutes: 30 + permissions: + contents: read + strategy: + fail-fast: false + matrix: + include: + - pdp: latest PDP image + env-suffix: pdp-latest + pdp-image: permitio/pdp-v2:latest + pdp-url: http://localhost:7766 + tests: tests/ + all-must-run: false + - pdp: cloud PDP + env-suffix: cloud-pdp + # No container: the tests call the hosted PDP. + pdp-image: '' + pdp-url: https://cloudpdp.api.permit.io + tests: tests/test_cloud_pdp_e2e.py + all-must-run: true + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install uv + uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + with: + version-file: "uv.lock" + python-version: "3.11.8" + enable-cache: true + # The entry the pydantic 2 lane of `pytest` saves: the same lock, + # group and Python, so the same packages. + cache-suffix: pydantic-v2 + + # As in `pytest`, values reach the shell through env:, never ${{ }}. + - name: Create the scratch environment + env: + PROJECT_ID: ${{ env.PROJECT_ID }} + ENV_NAME: ${{ env.ENV_NAME }} + RUN_ID: ${{ github.run_id }} + RUN_ATTEMPT: ${{ github.run_attempt }} + ENV_SUFFIX: ${{ matrix.env-suffix }} + PROJECT_API_KEY: ${{ secrets.PROJECT_API_KEY }} + run: | + set -euo pipefail + ENV_KEY="${ENV_NAME}-${RUN_ID}-${RUN_ATTEMPT}-${ENV_SUFFIX}" + echo "ENV_KEY=$ENV_KEY" >> "$GITHUB_ENV" + + response=$(curl -sS -X POST \ + "https://api.permit.io/v2/projects/${PROJECT_ID}/envs" \ + -H "Authorization: Bearer ${PROJECT_API_KEY}" \ + -H 'Content-Type: application/json' \ + -d "{\"key\": \"${ENV_KEY}\", \"name\": \"${ENV_KEY}\"}") + + ENV_ID=$(echo "$response" | jq -r '.id') + if [ -z "$ENV_ID" ] || [ "$ENV_ID" = "null" ]; then + echo "::error title=Env creation failed::Could not create the scratch environment." + exit 1 + fi + echo "ENV_ID=$ENV_ID" >> "$GITHUB_ENV" + echo "New env created with key: $ENV_KEY" + + - name: Fetch the scratch environment's API key + env: + PROJECT_ID: ${{ env.PROJECT_ID }} + ENV_ID: ${{ env.ENV_ID }} + PROJECT_API_KEY: ${{ secrets.PROJECT_API_KEY }} + run: | + set -euo pipefail + response=$(curl -sS -X GET \ + "https://api.permit.io/v2/api-key/${PROJECT_ID}/${ENV_ID}" \ + -H "Authorization: Bearer ${PROJECT_API_KEY}") + + ENV_API_KEY=$(echo "$response" | jq -r '.secret') + if [ -z "$ENV_API_KEY" ] || [ "$ENV_API_KEY" = "null" ]; then + echo "::error title=API key fetch failed::Could not read the scratch environment's key." + exit 1 + fi + # Mask before export so the key can never surface in the job log. + echo "::add-mask::$ENV_API_KEY" + echo "ENV_API_KEY=$ENV_API_KEY" >> "$GITHUB_ENV" + + # Prints the digest :latest resolved to, which names the PDP release a + # failure here is about (look it up on Docker Hub). + - name: Start the PDP + if: matrix.pdp-image != '' + env: + ENV_API_KEY: ${{ env.ENV_API_KEY }} + PDP_IMAGE: ${{ matrix.pdp-image }} + run: | + set -euo pipefail + docker run -d --name permit-pdp \ + -p 7766:7000 \ + -e PDP_API_KEY="${ENV_API_KEY}" \ + -e PDP_DEBUG=true \ + "${PDP_IMAGE}" + docker image inspect --format '{{join .RepoDigests ", "}}' "${PDP_IMAGE}" + + - name: Install dependencies + run: uv sync --locked --group pydantic-v2 + + - name: Show installed packages + run: uv pip list + + # The same wait as in `pytest`, for the same reasons. + - name: Wait for the PDP + if: matrix.pdp-image != '' + run: | + set -uo pipefail + for i in $(seq 1 300); do + if curl -sf http://localhost:7766/healthy > /dev/null 2>&1; then + echo "PDP healthy after ${i}s" + exit 0 + fi + sleep 1 + done + echo "::error title=PDP did not become healthy::/healthy never returned 200 within 300s" + exit 1 + + - name: Test with pytest + env: + PDP_URL: ${{ matrix.pdp-url }} + API_TIER: prod + ORG_PDP_API_KEY: ${{ env.ENV_API_KEY }} + PROJECT_PDP_API_KEY: ${{ env.ENV_API_KEY }} + PDP_API_KEY: ${{ env.ENV_API_KEY }} + TESTS: ${{ matrix.tests }} + run: >- + uv run --no-sync pytest -s --cache-clear + --junitxml="${RUNNER_TEMP}/junit.xml" "${TESTS}" + + # Cloud leg only. tests/test_cloud_pdp_e2e.py skips itself unless PDP_URL is + # the cloud PDP. If that check and this leg's PDP_URL ever disagree, every + # test skips and the leg passes having tested nothing. The whole suite, + # which the latest-PDP leg runs, has tests that skip by design. + - name: Check that no test was skipped + if: matrix.all-must-run + run: | + set -euo pipefail + uv run --no-sync python - "${RUNNER_TEMP}/junit.xml" <<'PY' + import sys + import xml.etree.ElementTree as ET + + suite = ET.parse(sys.argv[1]).getroot().find("testsuite") + tests, skipped = int(suite.get("tests")), int(suite.get("skipped")) + if skipped: + sys.exit(f"{skipped} of {tests} tests were skipped; this leg must run them all") + print(f"all {tests} tests ran") + PY + + - name: PDP logs + if: failure() && matrix.pdp-image != '' + run: | + docker logs permit-pdp 2>&1 \ + | grep -Ev 'GET /health|Health check failed: horizon' \ + | tail -300 || true + + - name: Stop the PDP + if: always() && matrix.pdp-image != '' + run: docker rm -f permit-pdp || true + + - name: Delete the scratch environment + if: always() + env: + PROJECT_ID: ${{ env.PROJECT_ID }} + ENV_ID: ${{ env.ENV_ID }} + PROJECT_API_KEY: ${{ secrets.PROJECT_API_KEY }} + run: | + set -uo pipefail + if [ -z "${ENV_ID:-}" ] || [ "${ENV_ID}" = "null" ]; then + echo "::warning::No ENV_ID recorded; nothing to delete." + exit 0 + fi + if ! curl -sS -f -X DELETE \ + "https://api.permit.io/v2/projects/${PROJECT_ID}/envs/${ENV_ID}" \ + -H "Authorization: Bearer ${PROJECT_API_KEY}"; then + leaked="Failed to delete environment ${ENV_ID}. Delete it by hand." + echo "::warning title=Scratch env leaked::${leaked}" + fi + # Offline suite on every supported Python. It needs no secrets and no PDP, so # it also runs on fork PRs. Kept apart from `pytest` above, whose name and # matrix are required status checks. compatibility: runs-on: ubuntu-24.04 + timeout-minutes: 15 permissions: contents: read strategy: @@ -315,8 +529,11 @@ jobs: # _sync_types.pyi. Neither is a .py file, so a packaging change can drop # them without any import failing. The artifacts are the same on every # leg, so one leg builds and checks them: the wheel, and the sdist, since - # a wheel built from the sdist holds only what the sdist does. - - name: Check the wheel and sdist ship their type information + # a wheel built from the sdist holds only what the sdist does. Nor may + # either one ship a package other than permit (permit 2.8.3's wheel + # installed a top-level `tests` package). The release build in + # python-sdk-publish.yml runs the same check; keep the two identical. + - name: Check the wheel and sdist contents if: matrix.python-version == '3.14' && matrix.deps == 'pydantic-v2' run: | set -euo pipefail @@ -327,22 +544,39 @@ jobs: import zipfile from pathlib import Path + REQUIRED = ["permit/py.typed", "permit/_sync_types.pyi"] + + + def check(artifact: Path, names: set[str], found: set[str], allowed: set[str]) -> None: + missing = [path for path in REQUIRED if path not in names] + if missing: + sys.exit(f"{artifact.name} is missing {missing}") + unexpected = sorted(found - allowed) + if unexpected: + sys.exit(f"{artifact.name} holds {unexpected}; only {sorted(allowed)} may ship") + print(f"{artifact.name} ships {' and '.join(REQUIRED)} and no package beside permit") + + dist = Path(sys.argv[1]) wheels = sorted(dist.glob("*.whl")) sdists = sorted(dist.glob("*.tar.gz")) if len(wheels) != 1 or len(sdists) != 1: found = [path.name for path in wheels + sdists] sys.exit(f"expected one wheel and one sdist in {dist}, found {found}") - required = ["permit/py.typed", "permit/_sync_types.pyi"] - contents = {wheels[0]: set(zipfile.ZipFile(wheels[0]).namelist())} - # Every sdist path starts with its top-level permit-/ directory. - with tarfile.open(sdists[0]) as sdist: - contents[sdists[0]] = {name.partition("/")[2] for name in sdist.getnames()} - for artifact, names in contents.items(): - missing = [path for path in required if path not in names] - if missing: - sys.exit(f"{artifact.name} is missing {missing}") - print(f"{artifact.name} ships {' and '.join(required)}") + wheel, sdist = wheels[0], sdists[0] + + # A wheel's top level is what lands in site-packages: permit/ and its + # permit-.dist-info, named after permit--.whl. + with zipfile.ZipFile(wheel) as wheel_file: + names = set(wheel_file.namelist()) + dist_info = "-".join(wheel.name.split("-")[:2]) + ".dist-info" + check(wheel, names, {name.split("/")[0] for name in names}, {"permit", dist_info}) + + # Every sdist path starts with its permit-/ directory. Below + # it, the files are metadata and docs, and the only directory is permit/. + with tarfile.open(sdist) as sdist_file: + names = {name.partition("/")[2] for name in sdist_file.getnames()} + check(sdist, names, {name.split("/")[0] for name in names if "/" in name}, {"permit"}) PY # Every test that needs credentials, the Permit API or a PDP is marked diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a1b5a5f..32ba583 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ uv run pre-commit install # lint, format, type-check and uv.lock checks on ev `uv sync` installs the SDK from this checkout in editable mode, so the tests and scripts import the working tree's `permit`. `.python-version` selects Python 3.11, the version the -end-to-end CI job runs on. The SDK itself supports Python 3.10 and later. +end-to-end CI jobs run on. The SDK itself supports Python 3.10 and later. The ruff, mypy and typos hooks run through `uv run --locked`, which syncs `.venv` to `uv.lock` before running the tool, so the versions in `uv.lock` are the only ones in play; the hooks fail @@ -130,29 +130,74 @@ uv run --only-dev pytest -c .github/scripts/pytest.ini \ ### End-to-end tests The tests marked `e2e` talk to a real Permit environment through a running PDP. `uv run -pytest` with no arguments runs the whole suite (`testpaths` is `tests/`). CI -(`.github/workflows/test.yml`) creates a scratch environment per run, starts a PDP container -for it, and sets: +pytest` with no arguments runs the whole suite (`testpaths` is `tests/`). + +CI (`.github/workflows/test.yml`) runs the e2e tests in four jobs. Each job creates its own +scratch environment in the CI project and deletes it when the job ends, whether the tests +passed or not: + +- `pytest (Pydantic pydantic<2.0.0)` and `pytest (Pydantic pydantic>=2.0.0)`, the required + checks, run the whole suite against a PDP container. Its image is `PINNED_PDP_IMAGE` at + the top of the workflow: `permitio/pdp-v2` pinned by version and digest, so a new PDP + release cannot fail a required check. +- `e2e (latest PDP image)` is not a required check. Once both `pytest` jobs pass, it runs + the whole suite on pydantic 2 against `permitio/pdp-v2:latest` and logs the digest + `:latest` resolved to. If it fails while `pytest` passes, the newest PDP release behaves + differently from the pinned one. +- `e2e (cloud PDP)` is not a required check. Once both `pytest` jobs pass, it runs + `tests/test_cloud_pdp_e2e.py` against the hosted cloud PDP, + `https://cloudpdp.api.permit.io`, with no container. Each test creates its own small RBAC + policy in the scratch environment, waits for the cloud PDP to apply it, and checks the + exact answers of `check`, `bulk_check`, `get_user_permissions` and `filter_objects`. The + module runs only against the cloud PDP and skips anywhere else, so this job fails if any + of its tests is skipped. + +The jobs set: - `PDP_API_KEY`: the scratch environment's API key. Every e2e test fails without it. -- `PDP_URL=http://localhost:7766`: the PDP. This is also the default when unset. +- `PDP_URL`: `http://localhost:7766`, the PDP container, or `https://cloudpdp.api.permit.io` + in `e2e (cloud PDP)`. When it is unset, `tests/test_cloud_pdp_e2e.py` uses the cloud PDP + and every other test `http://localhost:7766`. - `API_TIER=prod`: sends the SDK's API calls to `https://api.permit.io`. - `ORG_PDP_API_KEY` and `PROJECT_PDP_API_KEY`: the same key, read by `tests/endpoints/test_envs.py`. Without `API_TIER=prod` (or an explicit `PDP_CONTROL_PLANE`), `tests/conftest.py` sends API -calls to `http://localhost:8000`. To reproduce CI locally with an environment-level API key: +calls to `http://localhost:8000`. To reproduce the required jobs locally with an +environment-level API key, on the PDP image they pin: ```sh -docker run -d --name permit-pdp -p 7766:7000 -e PDP_API_KEY="$PDP_API_KEY" \ - permitio/pdp-v2:latest +PDP_IMAGE=$(grep -Eo 'permitio/pdp-v2:[0-9.]+@sha256:[0-9a-f]{64}' .github/workflows/test.yml) +docker run -d --name permit-pdp -p 7766:7000 -e PDP_API_KEY="$PDP_API_KEY" "$PDP_IMAGE" PDP_URL=http://localhost:7766 API_TIER=prod \ ORG_PDP_API_KEY="$PDP_API_KEY" PROJECT_PDP_API_KEY="$PDP_API_KEY" \ uv run pytest -s --cache-clear tests/ ``` +Set `PDP_IMAGE=permitio/pdp-v2:latest` instead to reproduce `e2e (latest PDP image)`. The +cloud PDP tests need no container. They send their API calls to `https://api.permit.io` +whatever `API_TIER` is, unless `PDP_CONTROL_PLANE` is set: + +```sh +PDP_URL=https://cloudpdp.api.permit.io uv run pytest tests/test_cloud_pdp_e2e.py +``` + The suite creates and deletes objects in that environment, so use a throwaway one. +### Moving the PDP pin + +Dependabot does not update `PINNED_PDP_IMAGE`. To move it to a new PDP release, first check +that `e2e (latest PDP image)` passed on that release: its `Start the PDP` step logs the +digest `:latest` resolved to. Then set `PINNED_PDP_IMAGE` to +`permitio/pdp-v2:@`, where `` is the digest of the release's +multi-arch image index: + +```sh +curl -s https://hub.docker.com/v2/repositories/permitio/pdp-v2/tags/ | jq -r .digest +``` + +Docker pulls by the digest; the tag only names it. + ## Regenerating the sync stubs The blocking client, `permit.sync.Permit`, wraps the async classes at runtime, which type @@ -255,6 +300,30 @@ uv build # sdist and wheel into dist/ `dist/` is the only build output; uv's build backend leaves no `build/` or `*.egg-info` directory behind. -Releasing is done by publishing a GitHub release, which runs -`.github/workflows/python-sdk-publish.yml` (build, then security scan, then PyPI). The release -tag sets the version. +## Releasing + +Publishing a GitHub release runs `.github/workflows/python-sdk-publish.yml`. It runs on +`published` only, so saving a draft publishes nothing. The release tag sets the version: +`vX.Y.Z` or `X.Y.Z`, optionally with a PEP 440 suffix such as `rc1` or `.post1`. Any other +tag, including the hyphenated `X.Y.Z-rc.N` form older releases used, fails the build. + +The workflow has three jobs, each of which runs only if the one before it passed: + +1. **Build distribution** builds the sdist and the wheel with the uv version and checksum + pinned in the workflow. It fails if either one lacks `permit/py.typed` or + `permit/_sync_types.pyi`, or ships a package other than `permit`: the wheel may hold only + `permit/` and its `.dist-info`, and the sdist no directory but `permit/`. Pull requests + run the same check: one leg of the `compatibility` job in `.github/workflows/test.yml` + builds both files and checks them with an identical script. +2. **Security Gate** scans the runtime dependency trees with `.github/scripts/audit-deps.sh` + and fails on any fixable HIGH or CRITICAL advisory. The report is kept as the + `release-dependency-audit` artifact for 90 days. +3. **Publish to PyPI** uploads the two files with PyPI trusted publishing, so the job needs + no PyPI token. PyPI accepts the upload because the `permit` project on pypi.org lists + repository `permitio/permit-python`, workflow `python-sdk-publish.yml` and environment + `pypi` as a trusted publisher. Renaming the workflow file or the environment needs the + same change on pypi.org first, or the next release cannot upload. PyPI does not check + which branch or tag the job ran from, so the `pypi` environment's deployment rules + (Settings, Environments) are what keep a branch that edits the workflow from uploading. + +None of the jobs uses the Actions cache, and each has a timeout. diff --git a/tests/test_abac_pdp.py b/tests/test_abac_pdp.py deleted file mode 100644 index 737f9b3..0000000 --- a/tests/test_abac_pdp.py +++ /dev/null @@ -1,101 +0,0 @@ -import os -from typing import Any - -import aiohttp -import pytest - -from permit import Permit, PermitConnectionError, TenantCreate, UserCreate - -CLOUD_PDP_URL = "https://cloudpdp.api.permit.io" - -# Every test in this module asserts what the CLOUD PDP does with a policy kind -# it does not implement: it answers 501 and the SDK turns that into -# PermitConnectionError. A full PDP container answers those same calls -# successfully, so the assertions are false there -- the tests are not merely -# slow or flaky off the cloud PDP, they are inapplicable. -# -# conftest's `permit_cloud` fixture resolves its address as -# os.getenv("PDP_URL", CLOUD_PDP_URL), so it only reaches the cloud PDP when -# PDP_URL is unset or already points there. CI sets PDP_URL to the local PDP -# sidecar (.github/workflows/test.yml), which means `permit_cloud` is a local -# PDP client there and these three tests cannot pass as written. Skipping on -# the same condition the fixture uses keeps them honest: they run where they -# are meaningful and are reported as skipped, with the reason, where they are -# not. -CONFIGURED_PDP_URL = os.getenv("PDP_URL", CLOUD_PDP_URL) - -pytestmark = [ - pytest.mark.e2e, - pytest.mark.skipif( - not CONFIGURED_PDP_URL.startswith(CLOUD_PDP_URL), - reason=( - f"cloud-PDP-only test: permit_cloud is configured against {CONFIGURED_PDP_URL}, " - f"not {CLOUD_PDP_URL}. Unset PDP_URL (or point it at the cloud PDP) to run these." - ), - ), -] - - -def abac_user(user: UserCreate) -> dict[str, Any]: - return user.dict(exclude={"first_name", "last_name"}) - - -async def test_abac_pdp_cloud_error(permit_cloud: Permit) -> None: - user_test = UserCreate( - key="maya@permit.io", - email="maya@permit.io", - first_name="Maya", - last_name="Barak", - attributes={"age": 23}, - ) - tesla = TenantCreate(key="tesla", name="Tesla Inc") - - with pytest.raises((PermitConnectionError, aiohttp.ClientError)) as exc_info: - await permit_cloud.check( - abac_user(user_test), - "sign", - { - "type": "document", - "tenant": tesla.key, - "attributes": {"private": False}, - }, - ) - assert isinstance(exc_info.value, PermitConnectionError) - - -async def test_get_user_permissions_cloud_error(permit_cloud: Permit) -> None: - user_test = UserCreate( - key="maya@permit.io", - email="maya@permit.io", - first_name="Maya", - last_name="Barak", - attributes={"age": 23}, - ) - - with pytest.raises((PermitConnectionError, aiohttp.ClientError)) as exc_info: - await permit_cloud.get_user_permissions( - user={ - "key": user_test.key, - "email": user_test.email, - "attributes": user_test.attributes, - }, - tenants=["default"], - resources=["Blog:dddddd"], - resource_types=["Blog"], - ) - assert isinstance(exc_info.value, PermitConnectionError) - - -async def test_filter_objects_cloud_error(permit_cloud: Permit) -> None: - user_test = {"key": "maya@permit.io", "email": "maya@permit.io", "attributes": {"age": 23}} - - test_resources: list[dict[str, Any]] = [ - {"type": "Blog", "key": "doc1", "context": {}, "attributes": {}, "tenant": "default"}, - {"type": "Document", "key": "doc2", "context": {}, "attributes": {}, "tenant": "default"}, - ] - - with pytest.raises((PermitConnectionError, aiohttp.ClientError)) as exc_info: - await permit_cloud.filter_objects( - user=user_test, action="read", context={}, resources=test_resources - ) - assert isinstance(exc_info.value, PermitConnectionError) diff --git a/tests/test_cloud_pdp_e2e.py b/tests/test_cloud_pdp_e2e.py new file mode 100644 index 0000000..1c59e7f --- /dev/null +++ b/tests/test_cloud_pdp_e2e.py @@ -0,0 +1,279 @@ +"""The SDK's decision calls against the hosted cloud PDP. + +Each test builds its own small RBAC policy in the environment the API key belongs to: a +resource type with two actions, a role that grants one of them, a tenant where the user +has that role and a second tenant where it has none. It waits for the cloud PDP to apply +the policy, then asserts the exact answers of ``check``, ``bulk_check``, +``get_user_permissions`` and ``filter_objects``. + +RBAC decides on the resource type and tenant alone, so the resources these tests ask +about need not exist as resource instances. +""" + +import asyncio +import functools +import os +import time +from collections.abc import AsyncIterator, Awaitable, Callable +from contextlib import AsyncExitStack +from dataclasses import dataclass +from typing import Any, Final, TypeVar + +import pytest + +from permit import Permit +from permit.exceptions import PermitApiError +from tests.utils import handle_cleanup_error, unique_key + +CLOUD_PDP_URL: Final[str] = "https://cloudpdp.api.permit.io" + +# conftest's `permit_cloud` fixture resolves its address as +# os.getenv("PDP_URL", CLOUD_PDP_URL), so it only reaches the cloud PDP when +# PDP_URL is unset or already points there. The jobs in +# .github/workflows/test.yml that start a PDP container set PDP_URL to it, and +# test_rbac_e2e.py already covers these decisions there. This module skips on +# the same condition the fixture uses, so it runs only against the cloud PDP and +# is reported as skipped, with the reason, everywhere else. The +# `e2e (cloud PDP)` job sets PDP_URL to the cloud PDP and fails if any test in +# this module is skipped. +CONFIGURED_PDP_URL: Final[str] = os.getenv("PDP_URL", CLOUD_PDP_URL) + +pytestmark = [ + pytest.mark.e2e, + pytest.mark.skipif( + not CONFIGURED_PDP_URL.startswith(CLOUD_PDP_URL), + reason=( + f"cloud-PDP-only test: permit_cloud is configured against {CONFIGURED_PDP_URL}, " + f"not {CLOUD_PDP_URL}. Unset PDP_URL (or point it at the cloud PDP) to run these." + ), + ), +] + +GRANTED_ACTION: Final[str] = "read" +DENIED_ACTION: Final[str] = "write" + +# Writes go to the Permit API and reach the cloud PDP asynchronously, and the +# environment is new to it. The bound is generous because it is only reached +# when the policy never arrives; polling returns as soon as it does. +PROPAGATION_TIMEOUT: Final[float] = 120.0 +POLL_INTERVAL: Final[float] = 1.0 + +T = TypeVar("T") + + +async def settled(fetch: Callable[[], Awaitable[T]], expected: T) -> T: + """Poll ``fetch`` until it returns ``expected``, for up to PROPAGATION_TIMEOUT seconds. + + An answer that includes an allow is polled for rather than asserted once: the cloud + PDP applies writes asynchronously, and one answer that reflects a write does not + guarantee the next one will. A deny is asserted once, since no stage of propagation + turns it into an allow. The last answer is returned either way, so the caller's + assertion reports the value the PDP gave. + """ + deadline = time.monotonic() + PROPAGATION_TIMEOUT + answer = await fetch() + while answer != expected and time.monotonic() < deadline: + await asyncio.sleep(POLL_INTERVAL) + answer = await fetch() + return answer + + +async def delete_quietly(delete: Callable[[], Awaitable[None]], description: str) -> None: + """Delete one object at teardown. A 404 means it is already gone, which is the goal.""" + try: + await delete() + except PermitApiError as error: + handle_cleanup_error(error, f"could not delete {description}") + + +@dataclass(frozen=True) +class CloudPolicy: + """The keys of one test's policy, all unique to it.""" + + resource: str + role: str + tenant: str + other_tenant: str + user: str + + @property + def granted_permission(self) -> str: + """The permission the role grants, as the PDP names it.""" + return f"{self.resource}:{GRANTED_ACTION}" + + def resource_in(self, tenant: str, key: str | None = None) -> dict[str, Any]: + """A resource of this policy's type in ``tenant``, optionally a single instance.""" + resource: dict[str, Any] = {"type": self.resource, "tenant": tenant} + if key is not None: + resource["key"] = key + return resource + + +@pytest.fixture +async def cloud_policy(permit_cloud: Permit) -> AsyncIterator[CloudPolicy]: + """Create one test's policy, wait until the cloud PDP applies it, and delete it after. + + Each delete is registered before the create it undoes, so teardown also removes an + object whose create call failed after the API had made it, and treats the 404 for one + it never made as success. Teardown runs in reverse order of registration. + """ + policy = CloudPolicy( + resource=unique_key("cloud-doc"), + role=unique_key("cloud-reader"), + tenant=unique_key("cloud-tenant"), + other_tenant=unique_key("cloud-other-tenant"), + user=unique_key("cloud-user"), + ) + api = permit_cloud.api + async with AsyncExitStack() as teardown: + teardown.push_async_callback( + delete_quietly, + functools.partial(api.resources.delete, policy.resource), + f"resource '{policy.resource}'", + ) + await api.resources.create( + { + "key": policy.resource, + "name": policy.resource, + "actions": {GRANTED_ACTION: {}, DENIED_ACTION: {}}, + } + ) + + teardown.push_async_callback( + delete_quietly, + functools.partial(api.roles.delete, policy.role), + f"role '{policy.role}'", + ) + await api.roles.create( + {"key": policy.role, "name": policy.role, "permissions": [policy.granted_permission]} + ) + + for tenant in (policy.tenant, policy.other_tenant): + teardown.push_async_callback( + delete_quietly, + functools.partial(api.tenants.delete, tenant), + f"tenant '{tenant}'", + ) + await api.tenants.create({"key": tenant, "name": tenant}) + + teardown.push_async_callback( + delete_quietly, + functools.partial(api.users.delete, policy.user), + f"user '{policy.user}'", + ) + await api.users.create({"key": policy.user}) + + assignment = {"user": policy.user, "role": policy.role, "tenant": policy.tenant} + teardown.push_async_callback( + delete_quietly, + functools.partial(api.users.unassign_role, assignment), + f"role assignment {assignment}", + ) + await api.users.assign_role(assignment) + + allowed = await settled( + lambda: permit_cloud.check( + policy.user, GRANTED_ACTION, policy.resource_in(policy.tenant) + ), + expected=True, + ) + assert allowed is True, ( + f"the cloud PDP did not allow '{policy.user}' to {GRANTED_ACTION} " + f"'{policy.resource}' in tenant '{policy.tenant}' within {PROPAGATION_TIMEOUT}s" + ) + yield policy + + +async def test_check(permit_cloud: Permit, cloud_policy: CloudPolicy) -> None: + policy = cloud_policy + instance = policy.resource_in(policy.tenant, key="doc-1") + + allowed = await settled( + lambda: permit_cloud.check(policy.user, GRANTED_ACTION, instance), expected=True + ) + + assert allowed is True + assert await permit_cloud.check(policy.user, DENIED_ACTION, instance) is False + assert ( + await permit_cloud.check( + policy.user, GRANTED_ACTION, policy.resource_in(policy.other_tenant, key="doc-1") + ) + is False + ) + + +async def test_bulk_check(permit_cloud: Permit, cloud_policy: CloudPolicy) -> None: + policy = cloud_policy + in_tenant = policy.resource_in(policy.tenant) + expected = [True, False, False, True] + + decisions = await settled( + lambda: permit_cloud.bulk_check( + [ + {"user": policy.user, "action": GRANTED_ACTION, "resource": in_tenant}, + {"user": policy.user, "action": DENIED_ACTION, "resource": in_tenant}, + { + "user": policy.user, + "action": GRANTED_ACTION, + "resource": policy.resource_in(policy.other_tenant), + }, + { + "user": policy.user, + "action": GRANTED_ACTION, + "resource": policy.resource_in(policy.tenant, key="doc-1"), + }, + ] + ), + expected=expected, + ) + + assert decisions == expected + + +async def test_get_user_permissions(permit_cloud: Permit, cloud_policy: CloudPolicy) -> None: + policy = cloud_policy + + async def tenant_grants() -> dict[str, dict[str, Any]]: + # The tenant's attributes are left out: how a PDP renders an empty set of + # them is not what this test is about. + permissions = await permit_cloud.get_user_permissions( + policy.user, tenants=[policy.tenant, policy.other_tenant] + ) + return { + key: { + "tenant": entry["tenant"]["key"], + "permissions": entry["permissions"], + "roles": entry.get("roles"), + } + for key, entry in permissions.items() + } + + # The cloud PDP also lists its built-in "tenant-association" role for a user who + # belongs to the tenant, after the roles assigned to them. + expected = { + f"__tenant:{policy.tenant}": { + "tenant": policy.tenant, + "permissions": [policy.granted_permission], + "roles": [policy.role, "tenant-association"], + } + } + + assert await settled(tenant_grants, expected=expected) == expected + + +async def test_filter_objects(permit_cloud: Permit, cloud_policy: CloudPolicy) -> None: + policy = cloud_policy + resources = [ + policy.resource_in(policy.tenant, key="kept-1"), + policy.resource_in(policy.other_tenant, key="dropped"), + policy.resource_in(policy.tenant, key="kept-2"), + ] + expected = [resources[0], resources[2]] + + kept = await settled( + lambda: permit_cloud.filter_objects(policy.user, GRANTED_ACTION, {}, resources), + expected=expected, + ) + + assert kept == expected + assert await permit_cloud.filter_objects(policy.user, DENIED_ACTION, {}, resources) == [] diff --git a/tests/test_offline_regressions.py b/tests/test_offline_regressions.py index 559b5ad..06a601c 100644 --- a/tests/test_offline_regressions.py +++ b/tests/test_offline_regressions.py @@ -14,6 +14,7 @@ from collections.abc import AsyncIterator, Sequence from datetime import datetime, timezone from decimal import Decimal +from operator import attrgetter from pathlib import Path from typing import Any, get_type_hints from uuid import UUID, uuid4 @@ -62,7 +63,7 @@ from permit.utils import pydantic_version from permit.utils.context import ContextStore from permit.utils.deprecation import deprecated -from tests.utils import FACTS +from tests.utils import FACTS, Call, call if sys.version_info >= (3, 11): import tomllib @@ -600,6 +601,55 @@ def test_check_query_context_is_optional() -> None: assert CheckQuery.__optional_keys__ == {"context"} +# How each decision call reports the status: "got an error: 501" (check), "status code: 501" +# (get_user_permissions and bulk_check, which filter_objects calls). The message also holds +# the PDP's URL, and a random httpserver port can contain 501, so a bare "501" proves nothing. +NOT_IMPLEMENTED = r"(?:status code|got an error): 501\b" + + +@pytest.mark.parametrize( + ("pdp_path", "target"), + [ + pytest.param( + "/allowed", + call("check", "user-1", "read", {"type": "document", "tenant": "t1"}), + id="check", + ), + pytest.param( + "/user-permissions", + call("get_user_permissions", "user-1", tenants=["t1"]), + id="get_user_permissions", + ), + pytest.param( + "/allowed/bulk", + call( + "filter_objects", + "user-1", + "read", + {}, + [{"type": "document", "key": "doc-1", "tenant": "t1"}], + ), + id="filter_objects", + ), + ], +) +async def test_a_pdp_answering_501_raises_a_connection_error_naming_the_status( + httpserver: HTTPServer, config: PermitConfig, pdp_path: str, target: Call +) -> None: + """A PDP that does not implement a decision call answers 501 (Not Implemented). + + The SDK must raise rather than return a decision, and say which status it got. + """ + httpserver.expect_request(pdp_path, method="POST").respond_with_json( + {"detail": "not implemented"}, status=501 + ) + + with pytest.raises(PermitConnectionError, match=NOT_IMPLEMENTED): + await attrgetter(target.path)(Permit(config))(*target.args, **target.kwargs) + + assert single_request(httpserver).path == pdp_path + + PYPROJECT = Path(__file__).resolve().parents[1] / "pyproject.toml" diff --git a/tests/test_rbac_e2e.py b/tests/test_rbac_e2e.py index 34827c9..ced13bd 100644 --- a/tests/test_rbac_e2e.py +++ b/tests/test_rbac_e2e.py @@ -437,6 +437,15 @@ async def test_permission_check_e2e( print_break() logger.info("testing list role assignments") + + # The PDP's list of role assignments can trail its decisions, so poll for it too. + async def assignment_listed() -> bool: + listed = await permit.pdp_api.role_assignments.list( + user_key=user.key, tenant_key=tenant.key + ) + return len(listed) == 1 + + await wait_until(assignment_listed, f"the PDP to list the role assignment of '{user.key}'") # scoped to this test's user and tenant: the environment is shared, so # the unfiltered list contains every other test's assignments too. assignments_returned: list[RoleAssignment] = await permit.pdp_api.role_assignments.list( @@ -490,6 +499,15 @@ async def test_permission_check_e2e( print_break() logger.info("testing get authorized users") + + # The PDP's authorized-users answer can trail its decisions, so poll for it too. + async def user_authorized() -> bool: + answer = await permit.authorized_users( + RESOURCE_CREATE_ACTION, {"type": document.key, "tenant": tenant.key} + ) + return user.key in answer.users + + await wait_until(user_authorized, f"the PDP to list '{user.key}' as authorized") authorized_users = await permit.authorized_users( RESOURCE_CREATE_ACTION, {"type": document.key, "tenant": tenant.key} ) @@ -699,6 +717,15 @@ async def test_local_facts_uploader_permission_check_e2e( print_break() logger.info("testing get authorized users") + + # The PDP's authorized-users answer can trail its decisions, so poll for it too. + async def user_authorized() -> bool: + answer = await permit.authorized_users( + RESOURCE_CREATE_ACTION, {"type": document.key, "tenant": tenant.key} + ) + return user.key in answer.users + + await wait_until(user_authorized, f"the PDP to list '{user.key}' as authorized") authorized_users = await permit.authorized_users( RESOURCE_CREATE_ACTION, {"type": document.key, "tenant": tenant.key} ) diff --git a/tests/test_rbac_e2e_sync.py b/tests/test_rbac_e2e_sync.py index 43a06c4..4054f81 100644 --- a/tests/test_rbac_e2e_sync.py +++ b/tests/test_rbac_e2e_sync.py @@ -304,6 +304,13 @@ def test_permission_check_e2e(sync_permit: SyncPermit) -> None: ) == [True, True, False] logger.info("testing list role assignments") + + # The PDP's list of role assignments can trail its decisions, so poll for it too. + def assignment_listed() -> bool: + listed = permit.pdp_api.role_assignments.list(user_key=user.key, tenant_key=tenant.key) + return len(listed) == 1 + + wait_until(assignment_listed, f"the PDP to list the role assignment of '{user.key}'") # scoped to this test's user and tenant: the environment is shared, so # the unfiltered list contains every other test's assignments too. assignments_returned: list[RoleAssignment] = permit.pdp_api.role_assignments.list( diff --git a/tests/test_user_invites_complete_e2e.py b/tests/test_user_invites_complete_e2e.py index 8950aea..fab27b9 100644 --- a/tests/test_user_invites_complete_e2e.py +++ b/tests/test_user_invites_complete_e2e.py @@ -14,8 +14,8 @@ ResourceInstanceCreate, ResourceInstanceRead, ResourceRead, - RoleCreate, - RoleRead, + ResourceRoleCreate, + ResourceRoleRead, TenantCreate, TenantRead, UserInviteStatus, @@ -32,7 +32,7 @@ def print_break() -> None: class SetupUserInvites(NamedTuple): created_resource: ResourceRead created_resource_instance: ResourceInstanceRead - created_role: RoleRead + created_role: ResourceRoleRead created_tenant: TenantRead to_create_invites: list[ElementsUserInviteCreate] @@ -61,7 +61,7 @@ async def setup_user_invites(permit: Permit) -> AsyncIterator[SetupUserInvites]: "first_name": "Test", "last_name": "User2", } - created_role: RoleRead | None = None + created_role: ResourceRoleRead | None = None created_tenant: TenantRead | None = None created_resource: ResourceRead | None = None created_resource_instance: ResourceInstanceRead | None = None @@ -113,16 +113,15 @@ async def setup_user_invites(permit: Permit) -> AsyncIterator[SetupUserInvites]: assert created_resource_instance.key == test_resource_instance.key logger.info(f"Created test resource instance: {created_resource_instance.key}") - # Create test role with permissions that match our resource actions - test_role = RoleCreate( + # The invites target a resource instance, so their role must be a role of that + # instance's resource: the API refuses to approve an invite whose role belongs to + # another resource (PER-15743). + test_role = ResourceRoleCreate( key=f"test_role_invites-{run_id.hex}", name="Test Role for Invites", - permissions=[ - f"{created_resource.key}:read", - f"{created_resource.key}:write", - ], # Use our resource actions + permissions=["read", "write"], ) - created_role = await permit.api.roles.create(test_role) + created_role = await permit.api.resource_roles.create(created_resource.key, test_role) assert created_role is not None assert created_role.key == test_role.key assert created_role.name == test_role.name @@ -172,9 +171,9 @@ async def setup_user_invites(permit: Permit) -> AsyncIterator[SetupUserInvites]: logger.warning(f"Failed to delete resource instance {instance_ident}: {e}") # Delete test role - if created_role is not None: + if created_role is not None and created_resource is not None: try: - await permit.api.roles.delete(created_role.key) + await permit.api.resource_roles.delete(created_resource.key, created_role.key) logger.info(f"Cleaned up role: {created_role.key}") except PermitApiError as e: if e.status_code != 404: # Ignore if already deleted