From bb543653776619c6c1c0a4628c23181c610e7014 Mon Sep 17 00:00:00 2001 From: Manush Murali Date: Thu, 15 Oct 2026 16:54:32 -0700 Subject: [PATCH 1/5] Build the release in CI, publish it where Kitmaker can be reached The v1.0.0 run failed at the first Portal request with "No address associated with hostname". Kitmaker's Portal is on NVIDIA's internal network, so a GitHub-hosted runner cannot resolve it, and no amount of credentials would have changed that. The workflow could never have published. So it no longer tries. `release` builds, attaches every distribution and the SBOM to the GitHub Release, and stops. Asking Kitmaker to publish from those asset URLs is a step a maintainer runs from inside the network, which is the same shape the sibling engine repository settled on. The approval on `pypi` stays, because attaching the distributions is what makes them public, and that is now the last gate before anything is visible. With the publish gone from CI there is no stored publishing credential and no environment secret left in this repository, and the Portal address and project ids are out of a public tree. That the removal does not retract them is worth saying plainly: they have been in the history since the publish landed, and a deletion today does not reach back. The hygiene check gains the pattern that would have caught the address when it was first written, since the service is named in SECURITY.md on purpose and only the unreachable host is the problem. Two behaviours change and are documented rather than left to be found. A full re-run after the build artifact expires now goes green while the release and `dist/` hold different bytes, because the digest comparison that used to stop it lives in the publish; the publish still refuses. And the release is created without Latest so it cannot advertise a version the index lacks, but with no earlier release to hold the position GitHub reports the newest one as latest anyway, so the first release reads as Latest from the moment it exists. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manush Murali --- .github/workflows/kitmaker_release.py | 524 ------------------ .github/workflows/publish.yml | 124 ++--- GOVERNANCE.md | 11 +- RELEASING.md | 100 ++-- SECURITY.md | 7 +- scripts/check_repo_hygiene.py | 5 + tests/test_kitmaker_release.py | 749 -------------------------- 7 files changed, 114 insertions(+), 1406 deletions(-) delete mode 100644 .github/workflows/kitmaker_release.py delete mode 100644 tests/test_kitmaker_release.py diff --git a/.github/workflows/kitmaker_release.py b/.github/workflows/kitmaker_release.py deleted file mode 100644 index e8b42ddce..000000000 --- a/.github/workflows/kitmaker_release.py +++ /dev/null @@ -1,524 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. -# All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -r"""Publish this release's distributions to PyPI through Kitmaker's Portal. - -Kitmaker does not accept an upload. It fetches each artifact from a URL and -publishes it itself, so the release workflow attaches the distributions to the -GitHub release first and this hands Portal those asset URLs. - -Two passes, and the split is the point. Every project is validated and dry-run -to a terminal state before any of them publishes, because the five -distributions pin each other by exact version: a client live on PyPI without -the engine it requires is worse than a release that never started. Portal -batches are sequential and not atomic, and a published file cannot be -recalled, so before the first upload is the only moment this stops cheaply. - -Anything already on PyPI at this version is verified and skipped rather than -resubmitted, so a run that failed halfway can be restarted. - -Run from the workflow with ``assets.tsv`` already written and ``dist/`` -downloaded. Exits non-zero on any failure; the workflow has no other way to -know this went wrong. -""" - -import hashlib -import http.client -import json -import os -import pathlib -import re -import sys -import time -import urllib.error -import urllib.request - -API = 'https://kitmaker-portal.nvidia.com/api/v0' -PYPI = 'https://pypi.org/pypi' - -# Ordered so nothing publishes before what it pins. The client requires api and -# pql exactly and reaches connectors and the engine through extras, so it goes -# last: a failed dependency leaves it absent rather than uninstallable. -PROJECTS = { - 6899: 'structured-data-models-api', - 6900: 'structured-data-models-pql', - 6901: 'structured-data-models-connectors', - 6902: 'structured-data-models-relational-engine', - 6898: 'structured-data-models-client', -} - -# The five states Portal documents, and no more. An unrecognised one is kept -# waiting rather than failed, which is what the docs ask for: continue bounded -# polling, record the value, escalate if progress stops, and never read an -# unfamiliar nonterminal value as success. -PENDING = {'pending', 'processing', 'in_progress'} -TERMINAL_OK = {'completed'} -TERMINAL_BAD = {'failed'} - -POLL_SECONDS = 30 -POLL_ATTEMPTS = 60 - -# Reads retry, submissions never: losing pass two to one connection reset is -# the half-published state the two passes exist to prevent, but a repeated POST -# could publish twice. -READ_RETRIES = 5 -READ_BACKOFF = 4 - -# Worth another go on a read. 404 only on `/status`, where it is normal for a -# second or two after a submit, before Portal knows the uuid; on -# `/projects/{id}` the same code means the project is not there and repeating -# it just delays the message. -RETRY_CODES = {408, 429} -RETRY_404_ON = '/status/' - -# A 200 carrying a CDN error page raises `ValueError` from `json.loads`, and a -# truncated body raises `IncompleteRead`, which is an `HTTPException` and not -# an `OSError`. Neither is an answer; both are worth retrying. -TRANSIENT = ( - urllib.error.URLError, - TimeoutError, - OSError, - ValueError, - http.client.HTTPException, -) - -# PyPI's JSON endpoint is CDN-cached and lags a publish by some seconds. -VERIFY_ATTEMPTS = 20 -VERIFY_SECONDS = 15 - - -TOKEN = os.environ['KITMAKER_API_TOKEN'] -PIC = os.environ['PIC'] - - -def canonical(name: str) -> str: - r"""Kitmaker's comparison: case insensitive, with ``.``, ``_`` and ``-`` - all equivalent.""" - return re.sub(r'[._-]+', '-', name).lower() - - -def distribution_of(filename: str) -> str: - r"""The distribution a built file belongs to. - - A wheel's first ``-`` separated field is its distribution, which cannot - itself contain ``-``. An sdist is ``{name}-{version}.tar.gz``, and a - normalised version contains no ``-`` either, so one split from the right - is enough. - """ - if filename.endswith('.whl'): - return filename.split('-')[0] - return filename.rsplit('-', 1)[0] - - -def version_of(filename: str) -> str: - r"""The version a built file carries. The build job has already refused to - continue unless all of them agree, so any one file answers.""" - if filename.endswith('.whl'): - return filename.split('-')[1] - return filename.rsplit('-', 1)[1].removesuffix('.tar.gz') - - -def call(method: str, path: str, body: dict | None = None) -> dict: - data = json.dumps(body).encode() if body is not None else None - attempts = READ_RETRIES if method == 'GET' else 1 - for attempt in range(1, attempts + 1): - request = urllib.request.Request( - f'{API}{path}', - data=data, - method=method, - headers={ - 'Authorization': f'Bearer {TOKEN}', - 'Content-Type': 'application/json', - }, - ) - try: - with urllib.request.urlopen(request, timeout=120) as response: - return json.loads(response.read() or b'{}') - except urllib.error.HTTPError as error: - # Decided before the body is read, because reading it can itself - # raise `IncompleteRead` on a truncated response, and that would - # escape past the sibling handler below without ever retrying. - # Most 4xx is an answer rather than a blip, so it is not repeated. - retryable = ( - error.code >= 500 - or error.code in RETRY_CODES - or (error.code == 404 and path.startswith(RETRY_404_ON)) - ) - if attempt < attempts and retryable: - time.sleep(READ_BACKOFF * attempt) - continue - try: - detail = error.read().decode(errors='replace')[:400] - except TRANSIENT: - detail = '' - raise SystemExit( - f'{method} {path} returned {error.code}: {detail}' - ) from error - except TRANSIENT as error: - if attempt < attempts: - time.sleep(READ_BACKOFF * attempt) - continue - raise SystemExit(f'{method} {path} failed: {error}') from error - raise SystemExit(f'{method} {path}: unreachable') - - -def on_pypi(name: str, version: str) -> dict[str, str] | None: - r"""What PyPI holds for this version as ``{filename: sha256}``, or ``None`` - if the version is absent. Public, so it answers for what a user gets.""" - url = f'{PYPI}/{name}/{version}/json' - for attempt in range(1, READ_RETRIES + 1): - try: - with urllib.request.urlopen(url, timeout=60) as response: - body = json.loads(response.read()) - return { - entry['filename']: entry['digests']['sha256'] - for entry in body.get('urls', []) - } - except urllib.error.HTTPError as error: - # 404 is PyPI's answer for a version it does not have, not a fault. - if error.code == 404: - return None - if attempt < READ_RETRIES and ( - error.code >= 500 or error.code in RETRY_CODES - ): - time.sleep(READ_BACKOFF * attempt) - continue - raise SystemExit( - f'PyPI {name} {version}: HTTP {error.code}' - ) from error - except TRANSIENT as error: - if attempt < READ_RETRIES: - time.sleep(READ_BACKOFF * attempt) - continue - raise SystemExit(f'PyPI {name} {version}: {error}') from error - raise SystemExit(f'PyPI {name} {version}: unreachable') - - -def wait_for(uuid: str, what: str) -> None: - r"""Poll to a terminal state. Running out of attempts is a failure, not a - pass: an unfinished release has told us nothing.""" - unfamiliar = set() - for _ in range(POLL_ATTEMPTS): - status = str(call('GET', f'/status/{uuid}').get('status', '')).lower() - if status in TERMINAL_OK: - print(f' {what}: {status}') - return - if status in TERMINAL_BAD: - raise SystemExit(f'{what} finished as {status} ({uuid})') - if status not in PENDING and status not in unfamiliar: - unfamiliar.add(status) - print(f' {what}: unrecognised status {status!r}, still waiting') - time.sleep(POLL_SECONDS) - seen = f', saw {sorted(unfamiliar)}' if unfamiliar else '' - raise SystemExit( - f'{what} never reached a terminal state after ' - f'{POLL_ATTEMPTS * POLL_SECONDS}s ({uuid}{seen})' - ) - - -def submit(project_id: int, name: str, urls: list[str], upload: bool) -> str: - response = call( - 'POST', - f'/projects/{project_id}/releases', - { - 'project_name': name, - 'payload': [ - { - 'pic': PIC, - 'job_type': 'wheel-release-job', - 'url': url, - 'upload': upload, - # Omitting this picks a strategy by size: under 1 GiB gives - # `upload-both`, at or above it gives `wheel-stub`, which - # cannot process an sdist. Everything here is far below the - # threshold, so the default would be right, but a size - # cliff between a wheel and its sdist is not something to - # leave implicit in a release path. - 'mirroring_strategy': 'upload-both', - } - for url in urls - ], - }, - ) - uuid = response.get('release_uuid') - if not uuid: - raise SystemExit(f'{name} returned no release_uuid: {response}') - return str(uuid) - - -def outstanding( - name: str, version: str, expected: dict[str, str] -) -> dict[str, str]: - r"""Which of this project's files are not on PyPI yet, as ``{filename: - sha256}``. - - Empty means the version is already published with the bytes this run built, - and is skipped; a partial version returns the rest, because PyPI accepts - files into a version one at a time. Two states raise instead, since neither - can be repaired by publishing more: a file held under different bytes, and a - file held that this run did not build. - """ - held = on_pypi(name, version) - if held is None: - return dict(expected) - differing = sorted( - filename - for filename, digest in expected.items() - if filename in held and held[filename] != digest - ) - if differing: - raise SystemExit( - f'{name} {version} is on PyPI with different bytes than this run ' - f'built: {differing}. PyPI does not allow a file to be replaced.' - ) - unknown = sorted(set(held) - set(expected)) - if unknown: - raise SystemExit( - f'{name} {version} is on PyPI with files this run did not build: ' - f'{unknown}.' - ) - return { - filename: digest - for filename, digest in expected.items() - if filename not in held - } - - -def verify(name: str, version: str, expected: dict[str, str]) -> None: - r"""Confirm PyPI holds exactly the bytes this run built. Kitmaker fetches - whatever is at the asset URL, so without this the only claim is that a job - reported success.""" - for _ in range(VERIFY_ATTEMPTS): - held = on_pypi(name, version) - if held is not None: - unknown = sorted(set(held) - set(expected)) - if unknown: - raise SystemExit( - f'{name} {version} on PyPI holds files this run did not ' - f'build: {unknown}' - ) - wrong = sorted( - filename - for filename, digest in expected.items() - if filename in held and held[filename] != digest - ) - if wrong: - raise SystemExit( - f'{name}: PyPI holds different bytes for {wrong}' - ) - if not set(expected) - set(held): - print( - f' verified on PyPI: {name} {version} ' - f'({len(expected)} files)' - ) - return - time.sleep(VERIFY_SECONDS) - raise SystemExit( - f'{name} {version} did not appear in full on PyPI after ' - f'{VERIFY_ATTEMPTS * VERIFY_SECONDS}s' - ) - - -def pypi_state(name: str, version: str, expected: dict[str, str]) -> str: - r"""What PyPI says about this project, in a phrase. - - Portal's own answer is not used here, and deliberately. Its batches are not - atomic, so a failure can follow files that already landed, and a rejection - is not proof that nothing did. Only the index knows, so the summary asks it - rather than repeating what Portal said. - - Every failure is caught, a malformed answer and a `SystemExit` from the - retry loop included: `SystemExit` is a `BaseException`, so catching - `Exception` alone would miss the one this module raises most. This runs from - a ``finally`` while an exception is already on its way out, and raising here - would replace the reason the release stopped with the reason the summary - could not be written, and leave no summary either. - """ - try: - held = on_pypi(name, version) - if held is None: - return 'not on PyPI' - wrong = sorted( - filename - for filename, digest in expected.items() - if filename in held and held[filename] != digest - ) - if wrong: - return f'on PyPI with UNEXPECTED bytes for {wrong}' - unknown = sorted(map(str, set(held) - set(expected))) - if unknown: - return f'on PyPI with files this run did not build: {unknown}' - absent = sorted(set(expected) - set(held)) - if absent: - return f'PARTLY on PyPI, missing {absent}' - return 'published' - except (Exception, SystemExit) as error: # noqa: BLE001 - see docstring - return f'UNKNOWN, could not ask PyPI ({error})' - - -def write_summary( - version: str, - digests: dict[str, dict[str, str]], - requested: list[str], - skipped: list[str], - partial: list[str], -) -> None: - r"""What is live and what is not, written whatever happens. - - A project whose upload was asked for is described by what PyPI holds now, - however that request ended, and so is one an earlier run left part-way: - `not reached` would otherwise hide a version that is already partly live. A - project neither of those is reported from what this run did. - """ - lines = [] - for name in PROJECTS.values(): - if name in skipped: - state = 'already published before this run' - elif name in requested or name in partial: - state = pypi_state(name, version, digests.get(name, {})) - else: - state = 'not reached' - lines.append(f'{name}: {state}') - text = '\n'.join(lines) + '\n' - try: - pathlib.Path('kitmaker-summary.txt').write_text(text) - except (Exception, SystemExit) as error: # noqa: BLE001 - see docstring - print(f'could not write kitmaker-summary.txt: {error}') - print(text, end='') - - -def main() -> None: - version = '' - by_name: dict[str, dict[str, str]] = {} - requested: list[str] = [] - skipped: list[str] = [] - partial: list[str] = [] - try: - built = { - path.name: hashlib.sha256(path.read_bytes()).hexdigest() - for path in pathlib.Path('dist').iterdir() - if path.name.endswith(('.whl', '.tar.gz')) - } - if not built: - raise SystemExit('dist/ holds no distributions') - # The workflow gate checks this too. This is the thing that submits, so - # it refuses as well: one version here is the version every file is - # published under. - versions = {version_of(filename) for filename in built} - if len(versions) != 1: - raise SystemExit( - f'distributions disagree on the version: {sorted(versions)}' - ) - version = versions.pop() - - # Only files this run built may be published. A rerun against an - # existing release can see assets left by an earlier one, so the - # release listing alone is not a safe description of this release. - assets: dict[str, str] = {} - stale: list[str] = [] - for line in pathlib.Path('assets.tsv').read_text().splitlines(): - if not line.strip(): - continue - filename, url, digest = (line.split('\t') + ['', ''])[:3] - if filename not in built: - continue - # Kitmaker fetches the asset, not the local file, and a published - # file cannot be recalled. An asset left by an earlier run that - # built different bytes would be published as this release and only - # then fail verification, which is far too late. The digest comes - # from the release listing, so this costs nothing. - if digest.removeprefix('sha256:') != built[filename]: - stale.append( - f'{filename} (release has {digest or "no digest"})' - ) - continue - assets[filename] = url - if stale: - raise SystemExit( - 'release assets are not the bytes this run built: ' - f'{sorted(stale)}. Re-run failed jobs rather than all jobs, ' - 'so the original artifact is reused.' - ) - - absent = set(built) - assets.keys() - if absent: - raise SystemExit(f'not attached to the release: {sorted(absent)}') - - urls: dict[int, dict[str, str]] = {} - for project_id, name in PROJECTS.items(): - mine = { - filename: url - for filename, url in assets.items() - if canonical(distribution_of(filename)) == canonical(name) - } - if not mine: - raise SystemExit(f'nothing in this release belongs to {name}') - urls[project_id] = mine - by_name[name] = {filename: built[filename] for filename in mine} - - claimed = sum(len(mine) for mine in urls.values()) - if claimed != len(assets): - raise SystemExit( - f'{len(assets) - claimed} built file(s) match no Portal project' - ) - - print( - f'{len(assets)} distributions at {version} ' - f'across {len(PROJECTS)} projects' - ) - - # What PyPI already holds, for every project, before anything is sent. - # Asking as each dry run came up meant a project an earlier run had - # left part-way went unexamined whenever an earlier project failed, and - # the summary then called it untouched. - todo: dict[int, tuple[str, list[str]]] = {} - for project_id, name in PROJECTS.items(): - pending = outstanding(name, version, by_name[name]) - if not pending: - print(f'already on PyPI, skipping: {name} {version}') - skipped.append(name) - continue - if len(pending) != len(by_name[name]): - partial.append(name) - todo[project_id] = ( - name, - sorted( - url - for filename, url in urls[project_id].items() - if filename in pending - ), - ) - - # Pass one: nothing publishes until every project has validated and - # every dry run has finished. - for project_id, (name, project_urls) in todo.items(): - actual = call('GET', f'/projects/{project_id}').get('name') - if actual != name: - raise SystemExit( - f'Portal project {project_id} is {actual!r}, not {name!r}' - ) - print(f'dry run: {name} ({len(project_urls)} files)') - wait_for( - submit(project_id, name, project_urls, False), f'dry run {name}' - ) - - if not todo: - print('every distribution is already published at this version') - return - - # Pass two: from here a failure can leave part of the release live, - # which is why the whole of pass one had to pass first. - for project_id, (name, project_urls) in todo.items(): - print(f'publishing: {name}') - # Recorded before it is asked for, because from this line on the - # files may be live whatever Portal goes on to say. - requested.append(name) - wait_for(submit(project_id, name, project_urls, True), name) - verify(name, version, by_name[name]) - finally: - write_summary(version, by_name, requested, skipped, partial) - - -if __name__ == '__main__': - sys.exit(main()) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 14c5d909e..b820b2132 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -7,22 +7,23 @@ # Kitmaker owns NVIDIA's PyPI organization, and this project's open source # review record commits to releasing through it. Kitmaker does not accept an # upload; it fetches an artifact from a URL and publishes it itself, to pypi.org -# and pypi.nvidia.com together. So the release here is two steps: attach every -# distribution to the GitHub Release, then ask Kitmaker's Portal to publish from -# those asset URLs. +# and pypi.nvidia.com together. This workflow's job is to produce those URLs: +# build every distribution and attach it to the GitHub Release. Asking Kitmaker +# to publish from them is a separate step that a person runs, because Kitmaker's +# Portal is on NVIDIA's internal network and a GitHub-hosted runner cannot +# resolve it, let alone reach it. RELEASING.md has that procedure. # # Two consequences worth stating rather than discovering. # -# There is a stored credential now. Kitmaker's Portal authenticates with a -# long-lived bearer token, which the previous Trusted Publishing setup -# deliberately avoided. `KITMAKER_API_TOKEN` must be an environment secret on -# `pypi` rather than a repository secret: approval gates environment secrets, -# so a repository secret would be readable from a workflow on any branch and -# the approval would protect nothing. It is read by one job, and it can -# publish only to the five Portal projects it owns. +# Nothing here can publish to an index. There is no stored publishing +# credential and no environment secret; the most this workflow can do is attach +# files to a Release. The approval on `pypi` still gates the job, because +# attaching the distributions is what makes them public and Kitmaker refuses +# private assets, so that approval is the last point before the artifacts are +# visible to anyone. # -# This workflow no longer signs what it ships. PEP 740 attestations are issued -# to whoever uploads, and that is Kitmaker now, so nothing here produces one +# This workflow does not sign what it ships. PEP 740 attestations are issued +# to whoever uploads, and that is Kitmaker, so nothing here produces one # naming this repository. Whether Kitmaker supplies its own is Kitmaker's to # say. A consumer is left with NVIDIA's ownership of the PyPI project, which # SECURITY.md states rather than dressing up. @@ -30,9 +31,9 @@ # `build` has `contents: read` to check the tree out and `actions: read` to ask # whether CI passed, and nothing more, so a compromised build dependency cannot # publish anything itself. It can still decide what gets published, because it -# is what produces the distributions. `release` holds `contents: write` and the -# Kitmaker token together, because the GitHub Release and the PyPI publish have -# to be one approval; it is the only job behind the protected environment. +# is what produces the distributions. `release` holds `contents: write` to +# create the Release and attach to it, and is the only job behind the +# protected environment. name: publish on: @@ -250,15 +251,11 @@ jobs: # the GitHub Release exists to give it public asset URLs. It refuses private # assets, so this cannot publish while the repository is internal. # - # Creating the release and publishing it are one gated job because the - # release cannot be a draft. Split, declining the approval still left a - # public release marked Latest, created by the ungated job that ran first. - # Together, declining means nothing was created at all. A publish that is - # approved and then fails does leave the release standing, which is - # deliberate: Kitmaker may already have fetched some of those assets, and - # `kitmaker-summary.txt` records, for each project an upload was requested - # for, what PyPI holds. The cost is the Kitmaker token - # sharing a job with `contents: write`. + # The Release is created behind the approval rather than before it. Creating + # it in an earlier ungated job would make every distribution public whether + # or not anyone approved, and it cannot be a draft instead, because a draft's + # asset URLs are private and Kitmaker could not fetch them. Declining the + # approval therefore means nothing was created at all. release: needs: build runs-on: ubuntu-latest @@ -266,29 +263,11 @@ jobs: permissions: contents: write steps: - # Before the download, because checkout cleans the working directory. - # `kitmaker_release.py` ships in the repository, not in the artifact. - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 with: name: distributions path: dist/ - # Before the release exists, because the release is the public part. A - # missing secret should stop this where nothing is visible yet. - - name: The Kitmaker credentials are present - env: - KITMAKER_API_TOKEN: ${{ secrets.KITMAKER_API_TOKEN }} - PIC: ${{ vars.KITMAKER_PIC }} - run: | - set -euo pipefail - : "${KITMAKER_API_TOKEN:?KITMAKER_API_TOKEN is not set}" - : "${PIC:?KITMAKER_PIC is not set}" - echo "both are set" - - name: Attach every distribution and the SBOM to the release env: GH_TOKEN: ${{ github.token }} @@ -301,10 +280,12 @@ jobs: # a tag that is not really there. if ! gh release view "$GITHUB_REF_NAME" --repo "$GITHUB_REPOSITORY" >/dev/null 2>&1; then # Not Latest yet. Kitmaker needs public asset URLs, so the - # release cannot be a draft and has to exist before the dry runs. - # A dry run that then fails would otherwise leave this standing as - # the current release for a version with nothing on PyPI. It is - # promoted once the publish has actually worked. + # release cannot be a draft and has to exist before anything is + # published from it. Marking it Latest here would advertise a + # version that is not on the index, so that is done by hand once + # the Kitmaker publish has worked. This only withholds the + # promotion: with no earlier release to stay Latest, GitHub still + # reports the newest non-draft release as the latest one. gh release create "$GITHUB_REF_NAME" \ --repo "$GITHUB_REPOSITORY" \ --title "$GITHUB_REF_NAME" \ @@ -338,42 +319,17 @@ jobs: gh release upload "$GITHUB_REF_NAME" "$path" --repo "$GITHUB_REPOSITORY" done - - name: Publish each distribution through Kitmaker - env: - GH_TOKEN: ${{ github.token }} - KITMAKER_API_TOKEN: ${{ secrets.KITMAKER_API_TOKEN }} - PIC: ${{ vars.KITMAKER_PIC }} + # The release is built and attached but nothing is on the index yet, + # and the person who approved this is the one who has to finish it. + - name: Say what is left to do run: | - set -euo pipefail - # `gh release view` lists whatever is attached, which on a rerun can - # include assets this run did not build. The artifact `build` handed - # over is the authority for what may be published, so the asset list - # is filtered down to exactly its filenames. - # `browser_download_url`, not `url`: in the REST response `url` is - # the API metadata endpoint, and fetching it without an - # `Accept: application/octet-stream` header returns JSON about the - # asset rather than the asset. Kitmaker is given only the URL. - # (`gh release view --json assets` maps its `.url` to the download - # URL, which is why this reads differently from it. The digest is - # only in the REST response, which is why this uses it.) - gh api "repos/$GITHUB_REPOSITORY/releases/tags/$GITHUB_REF_NAME" \ - --jq '.assets[] | "\(.name)\t\(.browser_download_url)\t\(.digest // "")"' \ - > assets.tsv - test -s assets.tsv - python3 .github/workflows/kitmaker_release.py - - # Only now, because until this point a failure could have left a - # release marked Latest for a version that never reached PyPI. - - name: Mark the release Latest, now that it is published - env: - GH_TOKEN: ${{ github.token }} - run: | - set -euo pipefail - gh release edit "$GITHUB_REF_NAME" --repo "$GITHUB_REPOSITORY" --latest - - # Into the job log, so a failed release says where it got to without - # anyone opening an artifact. The GitHub Release body is the changelog - # entry and is not touched. - - name: Print what each project ended up with - if: always() - run: cat kitmaker-summary.txt 2>/dev/null || true + { + echo "### Attached, not published" + echo + echo "\`$GITHUB_REF_NAME\` is on the Releases page with every" + echo "distribution and the SBOM attached, and nothing has reached" + echo "PyPI. Publishing is a separate step that runs from inside" + echo "NVIDIA's network. RELEASING.md has the procedure." + echo + echo "Mark this release Latest once the index has all five." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 3cd056129..cd94cbd29 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -46,11 +46,12 @@ Breaking changes are avoided within a major version. Where one is unavoidable, it is called out in [CHANGELOG.md](CHANGELOG.md) under a heading that says so, with the migration in the same entry. -Publishing is automated. A version tag on a commit that is on `main` and -whose CI passed builds every distribution, attaches it to the GitHub -Release, and asks NVIDIA's Kitmaker service to publish those artifacts to -PyPI. [RELEASING.md](RELEASING.md) describes what a maintainer does; `.github/workflows/publish.yml` is what -actually runs. +Building a release is automated. A version tag on a commit that is on `main` +and whose CI passed builds every distribution and attaches it to the GitHub +Release. Publishing those artifacts to PyPI is a separate step a maintainer +runs, because NVIDIA's Kitmaker service, which owns the PyPI projects, is not +reachable from a public CI runner. [RELEASING.md](RELEASING.md) describes what +a maintainer does; `.github/workflows/publish.yml` is what actually runs. ## Changing this document diff --git a/RELEASING.md b/RELEASING.md index 635b68975..11761527e 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -76,8 +76,9 @@ release missing it publishes an extra nobody can install. It then generates the SBOM, and takes the release's notes from the changelog's entry for that version, refusing a tag that has none: notes that read like a diff are fixable afterwards and a distribution on PyPI is not, so both happen before anything is -uploaded. It attaches every distribution and the SBOM to the GitHub Release for the -tag, and then asks Kitmaker's Portal to publish from those asset URLs. +uploaded. It attaches every distribution and the SBOM to the GitHub Release for +the tag, and stops there. **A green workflow means the artifacts are built and +public, not that anything is on PyPI.** **Publishing goes through Kitmaker, not straight to PyPI.** Kitmaker owns NVIDIA's PyPI organization and this project's OSRB record commits to releasing @@ -87,30 +88,36 @@ accepts an Artifactory path or a GitHub release asset, and it refuses private GitHub assets, so **a release cannot publish while the repository is internal**. -One Portal project exists per distribution, because Kitmaker matches the -project name against the distribution name parsed from each filename. Their -ids are in `.github/workflows/kitmaker_release.py`, and each is checked against -the name it should carry before anything is sent, so a renamed or reassigned id -fails early rather than being trusted. Every project that has something left to publish is validated -and dry-run to a terminal state before *any* of them publishes, because the five -distributions pin each other by exact version and a client live without its -engine is worse than a release that never started. Kitmaker batches are -sequential and not atomic, and a published file cannot be recalled. - -The GitHub Release is created in the same job that publishes, behind the -protected `pypi` environment. A release cannot be a draft, because Kitmaker -needs a public URL, so creating it in an earlier ungated job would make it -public before anyone approved the upload. - -**There is a stored credential.** `KITMAKER_API_TOKEN` is a long-lived bearer -token, read only by the `release` job behind the protected `pypi` environment. -It must be an **environment** secret on `pypi`, not a repository secret: -approval gates environment secrets, so a repository secret is readable by a -workflow on any branch and the approval protects nothing. The previous Trusted -Publishing setup avoided a stored token; this is the cost of publishing through -Kitmaker. - -**This workflow no longer signs what it ships.** PEP 740 attestations go to +**The publish is run by a person, from inside NVIDIA's network.** Kitmaker's +Portal is not on the public internet, so a GitHub-hosted runner cannot resolve +it, and no workflow here can do this step. The repository holds no Portal +address, project id or publishing credential for the same reason: none of them +belongs in a public repository, and nothing in CI has a use for them. The +procedure and the script live with the release owner, outside this repository. + +What that procedure has to do, so that a replacement is held to it: one Portal +project exists per distribution, because Kitmaker matches the project name +against the distribution name parsed from each filename, and each id is checked +against the name it should carry before anything is sent, so a renamed or +reassigned id fails early rather than being trusted. Every project that has +something left to publish is validated and dry-run to a terminal state before +*any* of them publishes, because the five distributions pin each other by exact +version and a client live without its engine is worse than a release that never +started. Kitmaker batches are sequential and not atomic, and a published file +cannot be recalled. + +The GitHub Release is created behind the protected `pypi` environment, because +creating it is what makes the distributions public. It cannot be a draft +instead, because a draft's asset URLs are private and Kitmaker could not fetch +them. Declining the approval means nothing was created at all. + +**Nothing in CI can publish to an index.** There is no stored publishing +credential and no environment secret; the most the `release` job can do is +attach files to a Release. The credential Kitmaker needs is a long-lived bearer +token, which is why it is held by the person running the publish rather than by +a workflow on a public repository. + +**This workflow does not sign what it ships.** PEP 740 attestations go to whoever uploads, and that is Kitmaker, so nothing here produces one naming this repository. SECURITY.md tells consumers what they can check instead. @@ -120,14 +127,16 @@ repository. SECURITY.md tells consumers what they can check instead. byte reproducible, so rebuilding produces a different wheel and sdist for the same version. The workflow will not clobber an already-attached asset, so a full re-run would leave the release holding the original bytes while `dist/` -holds new ones. The publish step compares each attached asset's digest against -what this run built and refuses before submitting anything, since Kitmaker -fetches the asset rather than the local file and a published file cannot be -recalled. "Re-run failed jobs" reuses the original `distributions` artifact, -which is what you want. - -`kitmaker-summary.txt`, printed at the end of the job, is the record to read -before doing anything else. It names each of the five projects. For any whose upload was requested it says +holds new ones. The publish compares each attached asset's digest against what +the run built and refuses before submitting anything, since Kitmaker fetches +the asset rather than the local file and a published file cannot be recalled. +"Re-run failed jobs" reuses the original `distributions` artifact, which is +what you want. + +### If the publish fails partway + +The summary the publish writes is the record to read before doing anything +else. It names each of the five projects. For any whose upload was requested it says what **PyPI** holds for this version, not what Portal reported: Portal's batches are not atomic, so a failure can follow files that already published, and a refusal is not proof that nothing did. Only the index knows. The other states @@ -159,13 +168,15 @@ that is part-published is reduced to the files still missing, since PyPI takes files into a version one at a time. A re-run after a partial publish therefore finishes what did not land. -**If the build artifact is gone**, neither re-run works, and they fail -differently. "Re-run failed jobs" stops at `actions/download-artifact`, since -an expired artifact cannot be downloaded. "Re-run all jobs" builds fresh, -non-reproducible files, finds the earlier assets still attached, and stops at -"release assets are not the bytes this run built". This happens once the -artifact's retention runs out, and after any change to the workflow that needs -a fresh build. +**If the build artifact is gone**, "Re-run failed jobs" stops at +`actions/download-artifact`, since an expired artifact cannot be downloaded. +"Re-run all jobs" builds fresh, non-reproducible files, finds the earlier +assets still attached, leaves them alone and reports each as already attached. +That run goes green with the release holding one set of bytes and `dist/` +another, so the workflow is not what catches this. The publish is, because it +compares every attached asset's digest against the artifact before it submits +anything. This happens once the artifact's retention runs out, and after any +change to the workflow that needs a fresh build. Recovery depends on the index: if nothing for this version is on PyPI, delete the release assets and run again; if anything is, the version is spent, so @@ -185,6 +196,13 @@ Check the five projects on pypi.org and confirm each shows NVIDIA organization ownership. Portal reporting that a release completed is not the same as the package being installable, so install it the way a user would. +Then mark the GitHub Release Latest. The workflow leaves it unmarked on +purpose, because until the publish has worked it would advertise a version the +index does not have. One exception is worth knowing: when there is no earlier +release to hold the position, GitHub reports the newest non-draft release as +the latest one regardless, so the first release reads as Latest from the +moment it is created. + ## Yanking A release that is wrong is yanked, not deleted. Deleting it breaks anyone who diff --git a/SECURITY.md b/SECURITY.md index 045b8328f..58d0b594d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -43,9 +43,10 @@ Kitmaker attestation instead is Kitmaker's to say; what this repository can tell you is that it does not sign these files itself. You can compare a downloaded file against the asset of the same name on the -corresponding GitHub release. The release workflow checks this itself: it will -not finish unless PyPI holds the same bytes it built and attached, and it never -replaces an asset it has already uploaded. So the two should agree. It is still +corresponding GitHub release. The release checks this itself: the publish will +not finish unless PyPI holds the same bytes that were built and attached, and +the workflow never replaces an asset it has already uploaded. So the two +should agree. It is still not proof of how either was built, and a release asset can be replaced by hand afterwards, so a mismatch is not by itself evidence of tampering on PyPI and a match is not evidence that this repository produced them. diff --git a/scripts/check_repo_hygiene.py b/scripts/check_repo_hygiene.py index 06897bdf2..4997e70a1 100644 --- a/scripts/check_repo_hygiene.py +++ b/scripts/check_repo_hygiene.py @@ -72,6 +72,11 @@ # A repository this one is not, and which no reader can open. re.compile(r'\bkumo-relational-client\b', re.IGNORECASE), re.compile(r'\bnv-shared-pypi\b'), + # Kitmaker's Portal is on the internal network, and its address shipped + # in a workflow script until the publish moved off CI. The service is + # named in SECURITY.md on purpose; the host a reader cannot reach is the + # part that does not belong here. + re.compile(r'kitmaker-portal|\bkm\.nvidia\.com\b', re.IGNORECASE), re.compile(r'\bnvbugs?\b', re.IGNORECASE), re.compile(r'\bnspect-\d', re.IGNORECASE), re.compile(r'\bcolossus\b', re.IGNORECASE), diff --git a/tests/test_kitmaker_release.py b/tests/test_kitmaker_release.py deleted file mode 100644 index 6cacb19d4..000000000 --- a/tests/test_kitmaker_release.py +++ /dev/null @@ -1,749 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. -# All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Checks on the release workflow, which no other suite can reach. - -The publish path runs once per release, against a service this repository -cannot call from CI, so the parts that can be read statically are read here: -the order projects publish in, the states the poller accepts, and the two -properties of the workflow job that a reviewer had to notice by eye. -""" - -from __future__ import annotations - -import ast -import hashlib -import importlib.util -import os -import pathlib -import re -import sys - -import pytest -import yaml - -ROOT = pathlib.Path(__file__).resolve().parent.parent -WORKFLOW = ROOT / '.github' / 'workflows' / 'publish.yml' -SCRIPT = ROOT / '.github' / 'workflows' / 'kitmaker_release.py' - - -@pytest.fixture(scope='module') -def kitmaker(): - """The publish script, which reads its credentials at import.""" - previous = {k: os.environ.get(k) for k in ('KITMAKER_API_TOKEN', 'PIC')} - os.environ['KITMAKER_API_TOKEN'] = 'test-token' - os.environ['PIC'] = 'nobody@nvidia.com' - try: - spec = importlib.util.spec_from_file_location( - 'kitmaker_release', SCRIPT - ) - module = importlib.util.module_from_spec(spec) - sys.modules['kitmaker_release'] = module - spec.loader.exec_module(module) - yield module - finally: - for key, value in previous.items(): - if value is None: - os.environ.pop(key, None) - else: - os.environ[key] = value - sys.modules.pop('kitmaker_release', None) - - -@pytest.fixture(scope='module') -def workflow(): - return yaml.safe_load(WORKFLOW.read_text()) - - -@pytest.fixture(scope='module') -def release_job(workflow): - return workflow['jobs']['release'] - - -def test_the_job_that_publishes_checks_out_the_script_it_runs(release_job): - """It runs `kitmaker_release.py`, which ships in the repository and not in - the build artifact, so without a checkout the step fails on a missing - file.""" - steps = release_job['steps'] - checkout = next( - ( - i - for i, s in enumerate(steps) - if 'actions/checkout' in s.get('uses', '') - ), - None, - ) - assert checkout is not None, ( - 'the publish job never checks out the repository' - ) - download = next( - i - for i, s in enumerate(steps) - if 'download-artifact' in s.get('uses', '') - ) - assert checkout < download, ( - 'checkout cleans the workspace, so it cannot follow the download' - ) - - -def test_nothing_is_published_or_released_outside_the_protected_environment( - release_job, -): - """The GitHub Release cannot be a draft, so creating it in an ungated job - would publish it before anyone approved the upload.""" - assert release_job.get('environment') == 'pypi' - run = ' '.join(s.get('run', '') for s in release_job['steps']) - assert 'gh release create' in run - assert 'kitmaker_release.py' in run - - -def test_an_attached_asset_is_never_replaced(release_job): - """These builds are not byte reproducible, so clobbering would leave the - GitHub asset and the PyPI file disagreeing.""" - commands = [ - line - for step in release_job['steps'] - for line in step.get('run', '').splitlines() - if not line.strip().startswith('#') - ] - assert not [line for line in commands if '--clobber' in line] - - -def test_the_client_publishes_after_everything_it_pins(kitmaker): - """It requires api and pql exactly and reaches connectors and the engine - through extras, so publishing it first would leave it live and - uninstallable if a later project failed.""" - order = list(kitmaker.PROJECTS.values()) - assert order[-1] == 'structured-data-models-client' - - -def test_every_distribution_has_a_portal_project(kitmaker): - packaged = { - p.parent.name for p in (ROOT / 'packages').glob('*/pyproject.toml') - } - assert set(kitmaker.PROJECTS.values()) == packaged - - -@pytest.mark.parametrize( - ('filename', 'distribution', 'version'), - [ - ( - 'structured_data_models_api-1.0.0-py3-none-any.whl', - 'structured_data_models_api', - '1.0.0', - ), - ( - 'structured_data_models_api-1.0.0.tar.gz', - 'structured_data_models_api', - '1.0.0', - ), - ( - 'structured_data_models_relational_engine-1.0.0-cp312-cp312-manylinux_2_28_x86_64.whl', - 'structured_data_models_relational_engine', - '1.0.0', - ), - ], -) -def test_a_built_file_names_its_distribution_and_version( - kitmaker, filename, distribution, version -): - assert kitmaker.distribution_of(filename) == distribution - assert kitmaker.version_of(filename) == version - - -def test_the_states_are_the_ones_portal_documents(kitmaker): - """Guessed state names were the original defect here. Portal documents - exactly these five, and nothing is invented alongside them.""" - assert kitmaker.PENDING == {'pending', 'processing', 'in_progress'} - assert kitmaker.TERMINAL_OK == {'completed'} - assert kitmaker.TERMINAL_BAD == {'failed'} - - -def test_an_unrecognised_status_keeps_waiting_and_is_reported( - kitmaker, monkeypatch, capsys -): - """Portal's docs say to continue bounded polling on a state the client does - not know, record it, and never read an unfamiliar nonterminal value as - success. Failing fast on it would stop a release that was still running.""" - monkeypatch.setattr( - kitmaker, 'call', lambda *a, **k: {'status': 'awaiting_approval'} - ) - monkeypatch.setattr(kitmaker.time, 'sleep', lambda _: None) - monkeypatch.setattr(kitmaker, 'POLL_ATTEMPTS', 3) - with pytest.raises(SystemExit, match='never reached a terminal state'): - kitmaker.wait_for('some-uuid', 'publish api') - out = capsys.readouterr().out - assert "unrecognised status 'awaiting_approval'" in out - assert out.count('unrecognised status') == 1, 'reported once, not per poll' - - -def test_a_documented_failure_is_reported_as_itself(kitmaker, monkeypatch): - monkeypatch.setattr(kitmaker, 'call', lambda *a, **k: {'status': 'failed'}) - monkeypatch.setattr(kitmaker.time, 'sleep', lambda _: None) - with pytest.raises(SystemExit, match='finished as failed'): - kitmaker.wait_for('some-uuid', 'publish api') - - -def test_an_sdist_goes_as_a_wheel_release_job_with_an_explicit_strategy( - kitmaker, monkeypatch -): - """`wheel-release-job` is the only job type Portal supports and it takes - sdists. The strategy is sent explicitly because omitting it chooses by - size, and the `wheel-stub` picked at or above 1 GiB cannot process one.""" - sent = {} - - def record(method, path, body=None): - sent.update(body or {}) - return {'release_uuid': 'u'} - - monkeypatch.setattr(kitmaker, 'call', record) - kitmaker.submit(1, 'a', ['https://example.invalid/a-1.0.0.tar.gz'], True) - entry = sent['payload'][0] - assert entry['job_type'] == 'wheel-release-job' - assert entry['mirroring_strategy'] == 'upload-both' - - -EXPECTED = {'a-1.0.0-py3-none-any.whl': 'aaa', 'a-1.0.0.tar.gz': 'bbb'} - - -def test_a_version_already_published_is_skipped(kitmaker, monkeypatch): - """What makes a rerun after a partial publish finish the rest.""" - monkeypatch.setattr(kitmaker, 'on_pypi', lambda *a: dict(EXPECTED)) - assert kitmaker.outstanding('a', '1.0.0', EXPECTED) == {} - - -def test_a_version_not_on_pypi_is_published_whole(kitmaker, monkeypatch): - monkeypatch.setattr(kitmaker, 'on_pypi', lambda *a: None) - assert kitmaker.outstanding('a', '1.0.0', EXPECTED) == EXPECTED - - -def test_a_partly_published_version_is_finished(kitmaker, monkeypatch): - """PyPI takes files into a version one at a time, so the rest can still go.""" - monkeypatch.setattr( - kitmaker, 'on_pypi', lambda *a: {'a-1.0.0-py3-none-any.whl': 'aaa'} - ) - assert kitmaker.outstanding('a', '1.0.0', EXPECTED) == { - 'a-1.0.0.tar.gz': 'bbb' - } - - -def test_a_version_published_from_other_bytes_is_refused(kitmaker, monkeypatch): - monkeypatch.setattr( - kitmaker, - 'on_pypi', - lambda *a: { - 'a-1.0.0-py3-none-any.whl': 'aaa', - 'a-1.0.0.tar.gz': 'different', - }, - ) - with pytest.raises(SystemExit, match='different bytes'): - kitmaker.outstanding('a', '1.0.0', EXPECTED) - - -def test_a_version_holding_files_we_did_not_build_is_refused( - kitmaker, monkeypatch -): - held = dict(EXPECTED) | {'a-1.0.0-cp312-cp312-linux_x86_64.whl': 'ccc'} - monkeypatch.setattr(kitmaker, 'on_pypi', lambda *a: held) - with pytest.raises(SystemExit, match='did not build'): - kitmaker.outstanding('a', '1.0.0', EXPECTED) - with pytest.raises(SystemExit, match='did not build'): - kitmaker.verify('a', '1.0.0', EXPECTED) - - -def test_publishing_is_not_complete_until_pypi_holds_the_built_bytes( - kitmaker, monkeypatch -): - monkeypatch.setattr(kitmaker, 'on_pypi', lambda *a: None) - monkeypatch.setattr(kitmaker.time, 'sleep', lambda _: None) - monkeypatch.setattr(kitmaker, 'VERIFY_ATTEMPTS', 2) - with pytest.raises(SystemExit, match='did not appear in full'): - kitmaker.verify('a', '1.0.0', EXPECTED) - - -@pytest.fixture -def release(kitmaker, tmp_path, monkeypatch): - """A whole release laid out the way the workflow hands it over, with every - call to Portal and PyPI answered locally.""" - - def files_of(name): - """A wheel and an sdist each, as a real release has: with one file per - project a part-published version could not be represented at all.""" - stem = name.replace('-', '_') - return { - f'{stem}-1.0.0-py3-none-any.whl': hashlib.sha256( - f'{name}-whl'.encode() - ).hexdigest(), - f'{stem}-1.0.0.tar.gz': hashlib.sha256( - f'{name}-sdist'.encode() - ).hexdigest(), - } - - def arrange(*, already_on_pypi=(), partly_on_pypi=()): - dist = tmp_path / 'dist' - dist.mkdir(exist_ok=True) - lines = [] - for name in kitmaker.PROJECTS.values(): - for filename, digest in files_of(name).items(): - (dist / filename).write_bytes( - f'{name}-{"whl" if filename.endswith(".whl") else "sdist"}'.encode() - ) - lines.append( - f'{filename}\thttps://example.invalid/{filename}\tsha256:{digest}' - ) - (tmp_path / 'assets.tsv').write_text('\n'.join(lines) + '\n') - - live = set(already_on_pypi) - half = set(partly_on_pypi) - - def on_pypi(name, version): - if name in half: - # The wheel landed, the sdist did not. - return { - f: d - for f, d in files_of(name).items() - if f.endswith('.whl') - } - if name not in live: - return None - return files_of(name) - - def submit(project_id, name, urls, upload): - if upload: - live.add(name) - return 'uuid' - - monkeypatch.setattr(kitmaker, 'on_pypi', on_pypi) - monkeypatch.setattr(kitmaker, 'submit', submit) - monkeypatch.setattr(kitmaker, 'wait_for', lambda *a: None) - monkeypatch.setattr( - kitmaker, - 'call', - lambda method, path, body=None: {'name': _name_for(kitmaker, path)}, - ) - monkeypatch.chdir(tmp_path) - return tmp_path / 'kitmaker-summary.txt' - - return arrange - - -def _name_for(kitmaker, path): - return kitmaker.PROJECTS[int(path.rsplit('/', 1)[1])] - - -def test_a_clean_release_reports_every_project_once(kitmaker, release): - """Each project belongs in exactly one line of the summary: the record of - what is live is what a half-finished release is read for.""" - summary = release() - kitmaker.main() - assert states(summary, kitmaker) == { - name: 'published' for name in kitmaker.PROJECTS.values() - } - - -def test_a_release_already_published_still_writes_a_summary(kitmaker, release): - summary = release(already_on_pypi=set(kitmaker.PROJECTS.values())) - kitmaker.main() - assert states(summary, kitmaker) == { - name: 'already published before this run' - for name in kitmaker.PROJECTS.values() - } - - -def test_a_project_live_but_unverified_is_not_reported_as_unpublished( - kitmaker, release, monkeypatch -): - """Portal reported success, so the files are on PyPI whatever verification - finds. Saying otherwise would send the next person to republish something - that is already there.""" - summary = release() - monkeypatch.setattr( - kitmaker, - 'verify', - lambda *a: (_ for _ in ()).throw(SystemExit('bad bytes')), - ) - with pytest.raises(SystemExit, match='bad bytes'): - kitmaker.main() - first = next(iter(kitmaker.PROJECTS.values())) - assert states(summary, kitmaker)[first] == 'published' - - -def states(summary, kitmaker): - """The summary as ``{project: state}``, asserting on the way that it names - each project once and names nothing else. A count of lines does not say - that: five lines naming one project would satisfy it.""" - lines = summary.read_text().strip().splitlines() - parsed = dict(line.split(': ', 1) for line in lines) - assert len(parsed) == len(lines), f'a project is named twice: {lines}' - assert set(parsed) == set(kitmaker.PROJECTS.values()) - return parsed - - -def test_a_failure_in_pass_one_still_records_what_happened( - kitmaker, release, monkeypatch -): - """A release that stopped before publishing is exactly when the summary is - read, and nothing was asked of PyPI, so nothing is claimed about it.""" - summary = release() - names = list(kitmaker.PROJECTS.values()) - - def mismatched(method, path, body=None): - project_id = int(path.rsplit('/', 1)[1]) - if kitmaker.PROJECTS[project_id] == names[1]: - return {'name': 'something-else'} - return {'name': kitmaker.PROJECTS[project_id]} - - monkeypatch.setattr(kitmaker, 'call', mismatched) - # Pinned to this failure: `match='not'` also matched `not attached to the - # release` and `dist/ holds no distributions`, so the test passed whatever - # went wrong first. - with pytest.raises(SystemExit, match="is 'something-else', not"): - kitmaker.main() - - assert states(summary, kitmaker) == {name: 'not reached' for name in names} - - -def test_a_preflight_failure_still_writes_a_summary( - kitmaker, release, tmp_path -): - """The checks that run before anything is submitted used to exit without - one, so a release that stopped there said nothing at all.""" - summary = release() - (tmp_path / 'dist' / 'structured_data_models_api-9.9.9.tar.gz').write_bytes( - b'x' - ) - with pytest.raises(SystemExit, match='disagree on the version'): - kitmaker.main() - assert states(summary, kitmaker) == { - name: 'not reached' for name in kitmaker.PROJECTS.values() - } - - -def test_every_project_is_accounted_for_exactly_once(kitmaker, release): - """The one property the summary exists to have, across a mixed run.""" - names = list(kitmaker.PROJECTS.values()) - summary = release(already_on_pypi={names[0], names[2]}) - kitmaker.main() - assert states(summary, kitmaker) == { - names[0]: 'already published before this run', - names[2]: 'already published before this run', - **{name: 'published' for name in (names[1], names[3], names[4])}, - } - - -@pytest.mark.parametrize('status', ['failed', 'cancelled', 'error']) -def test_a_portal_failure_after_the_files_landed_says_they_are_live( - kitmaker, release, monkeypatch, status -): - """Portal's batches are not atomic, so a failure can follow files that - already published. The summary asks PyPI rather than repeating the status, - which is what stops the next person republishing something that is there.""" - names = list(kitmaker.PROJECTS.values()) - summary = release() - real = kitmaker.wait_for - - def fail_after_publishing(uuid, what): - if what == names[1]: - raise SystemExit(f'{what} finished as {status} (uuid)') - return real(uuid, what) - - monkeypatch.setattr(kitmaker, 'wait_for', fail_after_publishing) - with pytest.raises(SystemExit, match=status): - kitmaker.main() - - assert states(summary, kitmaker)[names[1]] == 'published' - - -def test_an_upload_that_published_nothing_says_so( - kitmaker, release, monkeypatch -): - """The other direction: Portal refused, PyPI has nothing, and the summary - should not leave it ambiguous.""" - names = list(kitmaker.PROJECTS.values()) - summary = release() - - real = kitmaker.submit - - def refuse(project_id, name, urls, upload): - if upload and name == names[1]: - raise SystemExit('POST /releases returned 400: rejected') - return real(project_id, name, urls, upload) - - monkeypatch.setattr(kitmaker, 'submit', refuse) - with pytest.raises(SystemExit, match='400'): - kitmaker.main() - - assert states(summary, kitmaker)[names[1]] == 'not on PyPI' - - -def test_a_partly_published_project_is_reported_as_partial( - kitmaker, release, monkeypatch -): - names = list(kitmaker.PROJECTS.values()) - summary = release() - monkeypatch.setattr(kitmaker, 'verify', lambda *a: None) - original = kitmaker.on_pypi - - def half(name, version): - held = original(name, version) - if held and name == names[1]: - return {} - return held - - monkeypatch.setattr(kitmaker, 'on_pypi', half) - kitmaker.main() - state = states(summary, kitmaker)[names[1]] - assert state.startswith('PARTLY on PyPI, missing ') - - -def test_distributions_that_disagree_on_the_version_are_refused( - kitmaker, release, tmp_path -): - """The workflow gate reads the wheels and the sdists; this is what submits, - so it reads every file too.""" - release() - (tmp_path / 'dist' / 'structured_data_models_api-9.9.9.tar.gz').write_bytes( - b'x' - ) - with pytest.raises(SystemExit, match='disagree on the version'): - kitmaker.main() - - -@pytest.mark.parametrize( - 'failure', - [ - ValueError('Expecting value: line 1 column 1 (char 0)'), - # `SystemExit` is a `BaseException`, so `except Exception` misses it, - # and it is what every read in this module raises when it gives up. - SystemExit('PyPI structured-data-models-api 1.0.0: timed out'), - ], - ids=['malformed answer', 'read gave up'], -) -def test_the_summary_survives_pypi_failing( - kitmaker, release, monkeypatch, failure -): - """It runs from a finally while an exception is already on its way out, so - raising here would replace the reason the release stopped and leave no - summary either.""" - names = list(kitmaker.PROJECTS.values()) - summary = release() - - def rubbish(name, version): - raise failure - - def stop(uuid, what): - # Only the publish, so pass one completes and the project is one the - # summary has to describe rather than one nothing reached. - if not what.startswith('dry run'): - raise SystemExit('Portal fell over') - - monkeypatch.setattr(kitmaker, 'outstanding', lambda n, v, e: dict(e)) - monkeypatch.setattr(kitmaker, 'wait_for', stop) - monkeypatch.setattr(kitmaker, 'on_pypi', rubbish) - - with pytest.raises(SystemExit, match='Portal fell over'): - kitmaker.main() - - assert states(summary, kitmaker)[names[0]].startswith( - 'UNKNOWN, could not ask PyPI' - ) - - -def emitted_states(): - """Every state the summary can print, read off the source rather than - listed here, so a new one cannot be added without this noticing.""" - tree = ast.parse(SCRIPT.read_text()) - functions = { - node.name: node - for node in ast.walk(tree) - if isinstance(node, ast.FunctionDef) - } - - def literal(node): - if isinstance(node, ast.Constant) and isinstance(node.value, str): - return node.value - if isinstance(node, ast.JoinedStr): - head = node.values[0] - if isinstance(head, ast.Constant): - return head.value - return None - - states = set() - for node in ast.walk(functions['pypi_state']): - if isinstance(node, ast.Return) and node.value is not None: - phrase = literal(node.value) - assert phrase, ( - f'pypi_state returns something unreadable: {node.lineno}' - ) - states.add(phrase) - for node in ast.walk(functions['write_summary']): - if isinstance(node, ast.Assign) and any( - isinstance(t, ast.Name) and t.id == 'state' for t in node.targets - ): - phrase = literal(node.value) - if phrase: - states.add(phrase) - return states - - -def documented_states(): - """The first column of the state table in RELEASING.md.""" - rows = re.findall( - r'^\| `([^`]+)` \|', (ROOT / 'RELEASING.md').read_text(), re.MULTILINE - ) - return set(rows) - - -def test_every_state_the_summary_can_print_is_documented(): - """RELEASING.md tells a releaser what to do per state, which only works if - it lists them all, and misleads if it lists one that cannot happen.""" - emitted = emitted_states() - documented = documented_states() - assert len(emitted) >= 8, f'only found {emitted}: the reader is broken' - - def normalise(phrases): - """The fixed part of each state, with the punctuation that introduces - its variable tail removed. Compared as sets rather than by substring: - `published` is inside `already published before this run`, which let a - missing row pass.""" - return { - phrase.rstrip() - .removesuffix('[...]') - .rstrip() - .rstrip(':(,') - .rstrip() - .removesuffix(' for') - .rstrip() - for phrase in phrases - } - - undocumented = normalise(emitted) - normalise(documented) - assert not undocumented, ( - f'states RELEASING.md does not explain: {undocumented}' - ) - - unreachable = normalise(documented) - normalise(emitted) - assert not unreachable, ( - f'RELEASING.md documents states that cannot happen: {unreachable}' - ) - - -def test_an_asset_that_is_not_what_this_run_built_is_refused( - kitmaker, release, tmp_path -): - """Kitmaker fetches the asset, not the local file, and a published file - cannot be recalled. An asset left by an earlier run that built different - bytes would be published as this release and only then fail verification, - which is after the part that cannot be undone.""" - release() - tsv = tmp_path / 'assets.tsv' - rows = [line.split('\t') for line in tsv.read_text().strip().splitlines()] - rows[1][2] = 'sha256:' + '0' * 64 - tsv.write_text('\n'.join('\t'.join(row) for row in rows) + '\n') - - with pytest.raises(SystemExit, match='not the bytes this run built'): - kitmaker.main() - - -def test_an_asset_with_no_digest_is_refused(kitmaker, release, tmp_path): - """An asset the API cannot vouch for is the dangerous case, not the safe - one.""" - release() - tsv = tmp_path / 'assets.tsv' - rows = [line.split('\t') for line in tsv.read_text().strip().splitlines()] - rows[1][2] = '' - tsv.write_text('\n'.join('\t'.join(row) for row in rows) + '\n') - - with pytest.raises(SystemExit, match='no digest'): - kitmaker.main() - - -def test_kitmaker_is_given_a_url_that_serves_the_file(release_job): - """In the REST response `url` is the API metadata endpoint, which answers - with JSON about the asset unless asked for octet-stream. Kitmaker is handed - only the URL, so it has to be the download one.""" - commands = ' '.join( - line - for step in release_job['steps'] - for line in step.get('run', '').splitlines() - if not line.strip().startswith('#') - ) - assert 'assets.tsv' in commands - assert '.browser_download_url' in commands - assert '\\(.url)' not in commands - - -def test_the_release_is_not_latest_until_it_has_published(release_job): - """Kitmaker needs public asset URLs, so the release cannot be a draft and - has to exist before the dry runs. A dry run that then failed would leave it - standing as the current release for a version with nothing on PyPI.""" - steps = release_job['steps'] - create = next( - i - for i, s in enumerate(steps) - if 'gh release create' in s.get('run', '') - ) - promote = next( - i - for i, s in enumerate(steps) - if '--latest' in s.get('run', '') - and 'gh release edit' in s.get('run', '') - ) - publish = next( - i - for i, s in enumerate(steps) - if 'kitmaker_release.py' in s.get('run', '') - ) - assert '--latest=false' in steps[create]['run'] - assert create < publish < promote, 'Latest must be set after publishing' - - -def test_the_credentials_are_checked_before_anything_is_created(release_job): - """A missing secret should stop this where nothing is public yet.""" - steps = release_job['steps'] - checked = next( - i - for i, s in enumerate(steps) - if 'KITMAKER_API_TOKEN is not set' in s.get('run', '') - ) - created = next( - i - for i, s in enumerate(steps) - if 'gh release create' in s.get('run', '') - ) - assert checked < created - - -def test_two_runs_for_one_tag_cannot_race(release_job, workflow): - """They would upload assets from two different builds to one release, and - each would then refuse the other's files as stale.""" - group = workflow['concurrency']['group'] - assert 'github.ref' in group - assert workflow['concurrency']['cancel-in-progress'] is False - - -def test_a_partial_project_is_described_even_if_the_run_stops_first( - kitmaker, release, monkeypatch -): - """PyPI is asked about every project before any dry run. Asking as each dry - run came up meant a project an earlier run had left part-way went - unexamined whenever an earlier project failed, and the summary then called - it untouched.""" - names = list(kitmaker.PROJECTS.values()) - last = names[-1] - summary = release(partly_on_pypi={last}) - - monkeypatch.setattr( - kitmaker, - 'call', - lambda method, path, body=None: {'name': 'something-else'}, - ) - with pytest.raises(SystemExit, match="is 'something-else', not"): - kitmaker.main() - - reported = states(summary, kitmaker) - assert reported[names[0]] == 'not reached' - assert reported[last].startswith('PARTLY on PyPI, missing '), ( - f'a version already partly on PyPI was reported as {reported[last]!r}' - ) From 4dd5272f6b0e90df32bfd3d348b259a0cc10585f Mon Sep 17 00:00:00 2001 From: Manush Murali Date: Thu, 15 Oct 2026 17:13:00 -0700 Subject: [PATCH 2/5] Say in the opening what the gated job does, which is not publish The intro still described one approval covering the GitHub Release and the PyPI upload together, and described the environment reviewers as holding the upload. Both were true of the workflow this change replaced. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manush Murali --- RELEASING.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index 11761527e..5d4b0f7a7 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -1,8 +1,11 @@ # Releasing -Five distributions, one version, published together. A release is a tag and -then an approval: the `pypi` environment has required reviewers, so the job -that creates the GitHub Release and publishes to PyPI waits for one. +Five distributions, one version, published together. A release is a tag, then +an approval, then a publish. The tag builds everything. The `pypi` environment +has required reviewers, so the job that creates the GitHub Release and attaches +those distributions waits for one. Publishing them to PyPI is a third step, +run by hand, because the service that owns those projects is not reachable +from a public CI runner. The tag has to sit on a commit that is on `main` and whose CI run passed. The publish workflow checks both before it builds anything, so a tag on an @@ -12,7 +15,8 @@ That check lives in the workflow, and a tag runs the workflow as it was at the tagged commit, so it cannot defend against a tag placed on a commit older than the check. Two repository settings do, and both are configured outside this tree: a tag ruleset limiting who may create `v*`, and required -reviewers on the `pypi` environment, which hold the upload itself. +reviewers on the `pypi` environment, which hold the artifacts back from being +published on the Releases page at all. ## Before tagging From 2d0439ec4acab30fd2b19acaa6e6b5b0595712c2 Mon Sep 17 00:00:00 2001 From: Manush Murali Date: Thu, 15 Oct 2026 17:16:05 -0700 Subject: [PATCH 3/5] Stop the build comment promising a publish this workflow no longer does Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manush Murali --- .github/workflows/publish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index b820b2132..04dc11a61 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -117,8 +117,8 @@ jobs: done # The engine pins its siblings by exact version, and at this point in - # a release those versions exist only in dist/, not on PyPI - # yet, and will not be until the `release` job runs. cibuildwheel's test + # a release those versions exist only in dist/, not on PyPI yet, and + # will not be until someone publishes them. cibuildwheel's test # environment resolves from PyPI unless told otherwise, so without this # the smoke test fails with "structured-data-models-api was not found" and the release stops # before it can publish anything. cibuildwheel copies the directory it From 84b6ea98fd782887dce717abc8787ae1551622a5 Mon Sep 17 00:00:00 2001 From: Manush Murali Date: Thu, 15 Oct 2026 17:20:40 -0700 Subject: [PATCH 4/5] Correct two comments that described the publish this workflow handed off The concurrency note described two runs refusing each other's files as stale. That refusal was in the publish script, so two runs now both succeed and the release keeps whichever build attached first. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manush Murali --- .github/workflows/codeql.yml | 2 +- .github/workflows/publish.yml | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index dbd7ad79e..ec6c29d9f 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -80,7 +80,7 @@ jobs: # the Python leg reads the tree. # # `--no-deps` because the engine pins three siblings by exact version - # and those versions are not on PyPI until the release publishes, so + # and those versions are not on PyPI until someone publishes them, so # resolving them fails outright. `ci.yml` works around that by installing # the siblings from the tree first, which this job has no use for: the # dependencies are never imported, and what CodeQL needs is the compiler diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 04dc11a61..a8f694f79 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -44,10 +44,12 @@ on: # Least privilege at the top: a job that needs more says so itself. permissions: {} -# Two runs for one tag, say a tag push and a `workflow_dispatch`, would upload -# assets from two different builds to one release, and each would then refuse -# the other's files as stale. Not cancel-in-progress: a run that has started -# publishing must finish and write its summary. +# Two runs for one tag, say a tag push and a `workflow_dispatch`, would build +# that version twice and attach the results to one release. Nothing would fail: +# the second run finds the first's assets already there and leaves them, so the +# release ends up holding one build while the other run's artifact holds +# different bytes for the same filenames. Not cancel-in-progress: a run that +# has started attaching has to finish, or the release keeps part of a build. concurrency: group: release-${{ github.ref }} cancel-in-progress: false From 84f21f0c6912b44eb5a7f0fbdc4898a3db9f8171 Mon Sep 17 00:00:00 2001 From: Manush Murali Date: Thu, 15 Oct 2026 17:25:52 -0700 Subject: [PATCH 5/5] Do not assert what the index holds from a job that never asks it A dispatch against a tag that was already published would have made the step summary say the opposite of the truth. It now says only what this workflow did, and points at RELEASING.md for who finishes the release rather than claiming the procedure is in the tree. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manush Murali --- .github/workflows/publish.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index a8f694f79..d241f34ff 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -11,7 +11,8 @@ # build every distribution and attach it to the GitHub Release. Asking Kitmaker # to publish from them is a separate step that a person runs, because Kitmaker's # Portal is on NVIDIA's internal network and a GitHub-hosted runner cannot -# resolve it, let alone reach it. RELEASING.md has that procedure. +# resolve it, let alone reach it. RELEASING.md says who runs that step and +# what it has to do; the procedure itself is not kept in a public tree. # # Two consequences worth stating rather than discovering. # @@ -321,17 +322,20 @@ jobs: gh release upload "$GITHUB_REF_NAME" "$path" --repo "$GITHUB_REPOSITORY" done - # The release is built and attached but nothing is on the index yet, - # and the person who approved this is the one who has to finish it. + # The release is built and attached, and the person who approved this is + # the one who has to finish it. What the index holds is deliberately not + # asserted here: this job never asks, and a dispatch against a tag that + # was already published would make any claim about it wrong. - name: Say what is left to do run: | { echo "### Attached, not published" echo echo "\`$GITHUB_REF_NAME\` is on the Releases page with every" - echo "distribution and the SBOM attached, and nothing has reached" - echo "PyPI. Publishing is a separate step that runs from inside" - echo "NVIDIA's network. RELEASING.md has the procedure." + echo "distribution and the SBOM attached. This workflow publishes" + echo "nothing to PyPI; that is a separate step, run from inside" + echo "NVIDIA's network. RELEASING.md says who runs it and what it" + echo "has to do." echo echo "Mark this release Latest once the index has all five." } >> "$GITHUB_STEP_SUMMARY"