fix(release): paginate the release list so the draft reaper sees every release - #625
Merged
Merged
Conversation
…y release The release list is paginated by the GitHub API. listDraftReleases read a single response with no per_page and no Link-header walk, so it saw only the 30 most recent releases and the stale-draft reaper never found superseded RC drafts on any repository with a longer history. Draft cleanup is best-effort, so the accumulation was silent. Walk rel="next" at 100 per page, bound the walk so a malformed link chain cannot spin, fail loudly rather than return a truncated list, and pin the next page to the API host since every request carries a bearer token. Route draft resolution by tag or SHA through the same listing. Pass --paginate to the two fleet tag lookups that scan for a commit SHA across all tags. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Pinning only the host left the bearer token exposed to the same attacker the host pin exists to stop: a Link of http://api.github.com keeps the host, passes the check, and sends the token in cleartext. Compare the scheme too. The comparison is relative to the configured base rather than a hardcoded https, so an enterprise deployment reached over plain http via GITHUB_API_URL advertises http links of its own and still matches. Only a scheme disagreeing with the base is rejected. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
listDraftReleasesissuedGET /releaseswith noper_pageand noLink-header pagination, so it only ever saw GitHub's default first 30 releases.Its consumer is
cleanupStaleDrafts, the reaper that deletes superseded RC drafts. On any repository with more than 30 releases (this one has 173 tags and four pages of releases), the drafts it exists to delete have sunk past page one, so the reaper found nothing and superseded drafts accumulated forever. It was a no-op in exactly the case that needs it.create()logs a cleanup failure as a warning rather than failing, so the truncation was doubly silent.Verified before fixing: a live
GETon this repo's releases returns exactly 30 items with arel="next"header.gh api repos/stablekernel/cascade/tagsreturns 30; with--paginateit returns 173.Existing tests could not catch this: every release stub encodes the whole corpus into one response, so an unpaginated caller reads as correct.
Fix
rel="next"atper_page=100until the API stops advertising a next page.Linktarget would hand that token to whoever set the header.findReleaseByTagOrSHA, which stopped at the first 100 releases, through the same listing.--paginateto thefleet-e2eandsuite-bootstrap-pinfallbacks that scan all tags for a commit SHA.Verification
Four new tests in
internal/release/list_pagination_test.gostage a stub that paginates for real: it honorsper_page, slices bypage, and emits a genuineLink: <...>; rel="next"header, omitting it on the last page. The corpus is 250 releases, which no single request can cover at any page size.Before the fix these fail, and the failure is the bug speaking: the lister returned 0 of 2 drafts and the reaper deleted nothing. Also covered: a mid-pagination error surfaces rather than truncating, and an unterminated link chain stops at the bound.
go build ./...,go test ./... -count=1(3367 pass),go test ./... -race -count=1,golangci-lint run ./...clean.e2ebuilds and vets.cascade verify --own-repo: no drift.go test ./internal/changelog/...passes post-commit.No e2e scenario: the harness would need to stage more than 30 releases to exercise the boundary, which is impractical and slow, and a scenario that stages fewer would assert nothing about pagination. The unit test with a real multi-page
Linkheader is the honest bar here.Touches the release path, so this is fleet-relevant.
Follow-up
Two swallowed-error defects of a different root cause are left out to keep this reviewable:
internal/reset/reset.go(status, _ := r.gitOutput(...)treats a git failure as "no changes") andinternal/version/command.go(baseSHA, _ = git.GetInitialCommit()proceeds with zero commits). Both are local-git error handling in unrelated packages, not pagination.