Add vcs remote type #162

Merged
benvin merged 4 commits from benvin/remote-vcs into master 2026-10-24 17:21:05 +11:00
Member

The generic github remote only allowlists URL patterns: every repo needs hand-written mutable patterns for branch archives, and a moved branch re-downloads the whole archive under a ref-named key.

  • add vcs package type for GitHub or Gitea (picked from base_url) tag, commit and release-download paths, cached immutable
  • redirect archive/refs/heads/<branch> to the commit archive, resolving the branch via the branch API with one shared, per-remote bounded lookup cache reused for mutable_ttl
  • cache missing branches for at least 60s and map API rate limits to 503 with Retry-After
  • reject any other path shape, escaping segments and denied redirect targets
  • add docs/vcs/remote.md, a UI type def and a vcs e2e caching case
The generic `github` remote only allowlists URL patterns: every repo needs hand-written mutable patterns for branch archives, and a moved branch re-downloads the whole archive under a ref-named key. - add `vcs` package type for GitHub or Gitea (picked from `base_url`) tag, commit and release-download paths, cached immutable - redirect `archive/refs/heads/<branch>` to the commit archive, resolving the branch via the branch API with one shared, per-remote bounded lookup cache reused for `mutable_ttl` - cache missing branches for at least 60s and map API rate limits to `503` with `Retry-After` - reject any other path shape, escaping segments and denied redirect targets - add `docs/vcs/remote.md`, a UI type def and a vcs e2e caching case
unkin-agent added 3 commits 2026-10-24 16:36:33 +11:00
Name only redirect-following clients verified in vcs docs
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
d15364cbd6
Author
Member
  • internal/provider/vcs/vcs.go (lookup) — unknown or invalid branches (404/422) are never cached and there is no singleflight, so any client can loop random branch names and each one costs an API call against the shared server GitHub credential (also used by the rpm/deb github providers) or the 60/h anonymous quota → cache negative results for a short TTL and collapse concurrent lookups per key.
  • internal/provider/vcs/vcs.go (refs map) — only grows: no eviction or max size, keyed by every repo/branch any client resolves when patterns is empty → cap entries (LRU or sweep expired on insert).
  • internal/provider/vcs/vcs.go (isGitHub) — duplicates generic.isGitHubHost and diverges: www.github.com / api.github.com base_urls are treated as Gitea and hit /api/v1/... (404 → "branch not found"), and a GitHub Enterprise host silently takes the Gitea path the same way → reuse the shared host check; reject or document non-github.com GitHub-style hosts instead of misreporting 404.
  • internal/provider/vcs/vcs.go (lookup status handling) — a 403/429 from the API is a generic 502 with no Retry-After, and with stale_on_error=false there is no fallback → map 429/403-with-rate-limit to 503/429 and pass Retry-After through.
  • internal/provider/vcs/vcs.go (New) — nit: lookup uses its own 30s client, ignoring the remote's upstream_*_timeout overrides → use the remote's timeouts.
  • internal/provider/vcs/vcs.go (lookup) — nit: GitHub commits/<name> also resolves tags and shas, so refs/heads/<tag> redirects to a tag's commit → query commits/heads/<branch>.
  • internal/provider/vcs/vcs_test.go — nit: no test for auth headers (Gitea Basic, GitHub Bearer), 403/429 with and without stale_on_error, cache growth bound, concurrent lookups, or that the redirected commit path is itself denied by blocklist/patterns through the engine; the TTL-expiry case uses MutableTTL: 0 rather than elapsed time.
- internal/provider/vcs/vcs.go (lookup) — unknown or invalid branches (404/422) are never cached and there is no singleflight, so any client can loop random branch names and each one costs an API call against the shared server GitHub credential (also used by the rpm/deb github providers) or the 60/h anonymous quota → cache negative results for a short TTL and collapse concurrent lookups per key. - internal/provider/vcs/vcs.go (refs map) — only grows: no eviction or max size, keyed by every repo/branch any client resolves when `patterns` is empty → cap entries (LRU or sweep expired on insert). - internal/provider/vcs/vcs.go (isGitHub) — duplicates `generic.isGitHubHost` and diverges: `www.github.com` / `api.github.com` base_urls are treated as Gitea and hit `/api/v1/...` (404 → "branch not found"), and a GitHub Enterprise host silently takes the Gitea path the same way → reuse the shared host check; reject or document non-github.com GitHub-style hosts instead of misreporting 404. - internal/provider/vcs/vcs.go (lookup status handling) — a 403/429 from the API is a generic 502 with no Retry-After, and with `stale_on_error=false` there is no fallback → map 429/403-with-rate-limit to 503/429 and pass Retry-After through. - internal/provider/vcs/vcs.go (New) — nit: lookup uses its own 30s client, ignoring the remote's `upstream_*_timeout` overrides → use the remote's timeouts. - internal/provider/vcs/vcs.go (lookup) — nit: GitHub `commits/<name>` also resolves tags and shas, so `refs/heads/<tag>` redirects to a tag's commit → query `commits/heads/<branch>`. - internal/provider/vcs/vcs_test.go — nit: no test for auth headers (Gitea Basic, GitHub Bearer), 403/429 with and without stale_on_error, cache growth bound, concurrent lookups, or that the redirected commit path is itself denied by `blocklist`/`patterns` through the engine; the TTL-expiry case uses `MutableTTL: 0` rather than elapsed time.
unkin-agent added 1 commit 2026-10-24 16:49:04 +11:00
Cache missing branches, bound and dedupe vcs branch lookups, map rate limits to 503
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
cff459abda
Author
Member
  • nit: internal/provider/vcs/vcs.go:186 — Retry-After is passed through uncapped, so an upstream reset or HTTP-date far ahead gives a huge value (a huge integer Retry-After can also overflow the Duration in syncretry.go:36) → clamp secs to e.g. min(secs, 600).
- nit: internal/provider/vcs/vcs.go:186 — Retry-After is passed through uncapped, so an upstream reset or HTTP-date far ahead gives a huge value (a huge integer Retry-After can also overflow the Duration in syncretry.go:36) → clamp secs to e.g. min(secs, 600).
benvin merged commit 2905ec99e4 into master 2026-10-24 17:21:05 +11:00
benvin deleted branch benvin/remote-vcs 2026-10-24 17:21:05 +11:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/artifactapi#162