Skip to content

ci: publish to PyPI from the static-IP GitHub-hosted runner - #400

Merged
justinwlin merged 3 commits into
mainfrom
ci/publish-static-runner
Oct 30, 2026
Merged

justinwlin merged 3 commits into
mainfrom
ci/publish-static-runner

Conversation

@justinwlin

@justinwlin justinwlin commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Moves the PyPI Publish job in release-please.yml from Blacksmith to runpod-4vCPU-ubuntu-2404, the org's GitHub-hosted runner with static, allow-listed IPs. PyPI trusted publishing is built for GitHub-hosted runners, and Blacksmith counts as self-hosted. The label is literal because the org variable RUNNER_4VCPU_UBUNTU_2404 isn't shared with this repo. A probe job on this branch confirmed the runner accepts jobs from runpod/flash. The probe was then removed.

This doesn't fix the current Release failure. Since #399 the token step asks for workflows: write, and the release app (RELEASE_APP_ID) isn't granted it: The permissions requested are not granted to this installation. An org owner has to add Workflows: Read and write to the app and accept it on the installation. Then rerun Release on main. That publishes 1.21.1, which also needs runpod-workers/flash#112 released first, because of #390.

@justinwlin
justinwlin marked this pull request as ready for review October 30, 2026 15:58

@OneAsterisk OneAsterisk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review, round 1

Verdict: Approved — a one-line runner move that does what the description says; the two notes below are non-blocking.
Approval gate: 0 Critical, 0 High, 1 Medium+Low → approved.

PR description vs code

Claim Status Where
PyPI Publish moves from Blacksmith to runpod-4vCPU-ubuntu-2404 ✅ release-please.yml:52
The label is the org's GitHub-hosted runner with static, allow-listed IPs ✅ GitHub-hosted: the probe log shows runner group runpod-4vcpu-ubuntu-2404 and image ubuntu-24.04 from actions/runner-images. ⚠️ Static-IP and allow-list config needs org admin to read (403 here); consistent with runpod/KaOS and runpod/spinnaker-platform calling it "the org's static-IP runner". job 124146750304
Trusted publishing is built for GitHub-hosted runners; Blacksmith counts as self-hosted ⚠️ Mostly. pypa/gh-action-pypi-publish's README: self-hosted runners are "best effort", "Trusted Publishing is expected to work on self-hosted runners", and publishing from GitHub-provided runners is "the recommended/supported way". Supported path, not a hard requirement. See nit. README, Non-goals
RUNNER_4VCPU_UBUNTU_2404 isn't shared with this repo ✅ repos/runpod/flash/actions/organization-variables returns 0 variables API
A probe confirmed the runner accepts jobs from runpod/flash, then was removed ✅ Run 41080143161's sibling runner-probe on 204c67f succeeded on that runner group; 2507107 deletes runner-probe.yml; head has only ci.yml, e2e.yml, release-please.yml run 41080143737
This doesn't fix the current Release failure (workflows: write not granted) ✅ The latest main run fails at the token step with exactly The permissions requested are not granted to this installation run 40991928237
An org owner must grant Workflows: Read and write, then rerun Release ⚠️ Plausible; the app installation is not readable from here
1.21.1 needs runpod-workers/flash#112 released first, because of #390 ✅ #390's warning names runpod-workers/flash#113 (merged 2026-10-27, on main since v1.6.0); #112 is the 1.6.1 release PR whose tag would include it. ⚠️ #112's changelog is dated 2026-10-23 and does not list #113, so release-please there has not regenerated it since.

Findings

Critical

None.

High

None.

Medium

None.

Low

  • Literal runner label instead of the org variable — release-please.yml:52. The org's own tooling treats a written-out label as an error: runpod/spinnaker-platform's check-workflow-runners.sh rejects any runs-on that is not ${{ vars.RUNNER_4VCPU_UBUNTU_2404 }} ("named by an organization variable, so the label changes in one place"), and runpod/KaOS and runpod/internal-workflows use the variable. If the runner group is renamed, the next release's publish job waits for a runner that never arrives. The body already explains why the literal is needed today. The fix is the same ask as the app permission: have the org admin share RUNNER_4VCPU_UBUNTU_2404 with runpod/flash, then switch to the variable. Fine to merge as-is and follow up.

Nits

  • release-please.yml:52 "trusted publishing needs that" overclaims; pypa says it is expected to work on self-hosted runners and GitHub-hosted is the supported path. Suggested wording inline.

Simplification opportunities

None; one-line change.

Verified clean

  • Workflow-level id-token: write (line 11) still applies to the job, and PyPI's publisher match (repository, release-please.yml, environment pypi-production) does not depend on the runner, so the OIDC exchange is unaffected.
  • actions/setup-python, astral-sh/setup-uv with enable-cache, and pypa/gh-action-pypi-publish are all in their supported configuration on a GitHub-hosted runner.
  • Build Package is skipped by guard, not failed: compute_should_build only fires for packaging paths (ci.yml:118-126), and this PR touches none. Every other check passes at 2507107.
  • Both commits use ci:, hidden from the changelog; the repo squash-merges.
  • The other three jobs stay on Blacksmith, which already works under the org IP allow list (#397), so nothing else changes.
  • Merging adds one more failing Release run on main until the app permission is granted; the body says so.

Questions for the author

  • Can the org-admin request for the release app's Workflows permission also include sharing RUNNER_4VCPU_UBUNTU_2404 with runpod/flash? That closes the Low above.
  • runpod-workers/flash#112 still describes main as of 2026-10-23 and omits #113. Is that repo's release automation also stalled? The tag would still include 95de97c9, but the changelog would not.

Reviewed by Claude (claude-fable-5-1) via the pr-review skill. Tokens: 290 in / 31,674 out / 653,971 cache read / 95,402 cache write = 781,337. Estimated cost: $3.66 at $10 / $50 / $0.25 / $20 per MTok (input / output / cache read / 1-hour cache write), as of posting. Round 1 of this PR. This is automated assistance; a human owns the final call.

Comment thread .github/workflows/release-please.yml Resolved
Comment thread .github/workflows/release-please.yml Resolved
Co-authored-by: Ben Papp <90720963+OneAsterisk@users.noreply.github.com>

@OneAsterisk OneAsterisk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review, round 2

Verdict: Approved — the only change since round 1 is the comment wording on line 52, applied from the round 1 suggestion; nothing new to raise.
Approval gate: 0 Critical, 0 High, 1 Medium+Low → approved. Round 1's approval at 2507107 stands; this round re-approves at ccbb51a.

Status of prior findings

# Finding Severity Status Evidence
R1-1 Literal runner label instead of ${{ vars.RUNNER_4VCPU_UBUNTU_2404 }} Low Not addressed (blocked on org admin, not the author) release-please.yml:52 still literal at ccbb51a; organization-variables for runpod/flash still returns 0
R1-2 Comment claimed trusted publishing needs a GitHub-hosted runner Nit Fixed ccbb51a applied the suggestion verbatim; thread resolved

Existing feedback

  • No human or bot comments on this PR; the two threads are round 1's own, answered above.

PR description vs code

Re-checked the claims that could have moved since round 1; the rest are unchanged and still hold.

Claim Status Where
PyPI Publish runs on runpod-4vCPU-ubuntu-2404 ✅ release-please.yml:52
The probe job was removed ✅ head has only ci.yml, e2e.yml, release-please.yml .github/workflows/
RUNNER_4VCPU_UBUNTU_2404 isn't shared with this repo ✅ still 0 org variables visible API
This doesn't fix the current Release failure ✅ the newest Release run on main (40991928237, 02:28Z today) still fails at the token step; nothing newer has run run 40991928237

Findings

None new. The diff since round 1 is one comment string on one line.

Simplification opportunities

None.

Verified clean

  • The runs-on label itself is byte-identical to the one the probe exercised, so the round 1 probe result still applies.
  • The new comment is valid YAML (space before #, no quotes to break).
  • ccbb51a's subject (Update .github/workflows/release-please.yml) has no conventional-commit type, so release-please ignores it in the squash body; nothing leaks into the changelog.
  • CI at ccbb51a: every check passes (run 41087170098). Build Package stays skipped by guard (should_build only fires for packaging paths); pre-check, quality-gates, and nix run in full mode on every PR regardless, as ci.yml:205,227,324 gate them.

Questions for the author

  • Same two from round 1, still open: can the org-admin ask bundle sharing RUNNER_4VCPU_UBUNTU_2404 with the Workflows permission? And is runpod-workers/flash#112's changelog going to pick up #113 before it ships?

Reviewed by Claude (claude-fable-5-1) via the pr-review skill. Tokens: 358 in / 11,206 out / 803,627 cache read / 62,849 cache write = 878,040. Estimated cost: $2.02 at $10 / $50 / $0.25 / $20 per MTok (input / output / cache read / 1-hour cache write), as of posting. Round 2 of this PR. This is automated assistance; a human owns the final call.

@justinwlin
justinwlin merged commit 0ec7c26 into main Oct 30, 2026
10 checks passed
@justinwlin
justinwlin deleted the ci/publish-static-runner branch October 30, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants