ci: resilient protoc install to fix flaky release-CDN 504s - #557
Conversation
|
Warning Review limit reached
More reviews will be available in 2 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (2)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
arduino/setup-protoc@v3.0.0 downloads protoc-23.4 from the GitHub release CDN (objects.githubusercontent.com), which intermittently returns HTTP 504. The asset download is plain CDN traffic — repo-token (already set on all four steps) only authenticates the GitHub API release lookup, not the download — so the existing token does not prevent the flake. It hits whichever protoc-needing job loses the lottery (lint / schema-drift / rust-unit / build-bin), failing unrelated PRs; when build-bin is the loser, the dependent e2e + coverage jobs are skipped, so a green-code PR shows red CI. Replace it with a local composite action .github/actions/setup-protoc that downloads the SAME pinned protoc 23.4 with hard retries (curl --retry plus an outer 5x loop, 15s backoff) and installs it under $HOME (no sudo), exposing it via $GITHUB_PATH and pinning $PROTOC for prost/tonic build scripts. Keeps the exact version (no build risk from a distro protoc that may lack proto3 'optional') and DRYs the four call sites behind one action. Refs #550 (whose instrumented build kept losing the 504 lottery).
b38cdf6 to
aefb179
Compare
Model rows have carried a source-IP allowlist since api7#557, and a routing group's members since AISIX-Cloud#1087. MCP servers and A2A agents carried none. An operator who restricted a model to a source network found the same restriction unavailable for the MCP server and the A2A agent sitting beside it in the same environment, with nothing to say why — the field simply was not there. `allowed_cidrs` now exists on both resources with the same shape and the same write-path validation the model field has. The gate itself is ONE function, `models::model::cidr_allows`, which all three resources call: its fail-closed handling of an unattributable caller and its IPv4-mapped-IPv6 canonicalisation were each a real bug fixed once, and a copy per resource would still have them. The two endpoints refuse differently, and the code says why at each site: - `/a2a/<agent>` answers 403. It already discloses that an agent exists by answering an ACL denial with 403, so hiding only the IP case buys nothing and would make two refusals for the same agent answer differently. - `/mcp/<server>` treats the server as ABSENT — 404 on the scoped route, and filtered out of the aggregated gateway entirely. Telling a caller a private MCP server exists but is not theirs hands them the tool inventory the allowlist exists to withhold, and filtering at gateway construction is what makes listing and calling agree; a gate applied only when a tool is invoked would still publish every tool name. Same existence fold `/v1/videos` applies to an ACL denial. An empty `allowed_cidrs` is not a deny-all — the field is opt-in, so a row carrying it without entries serves exactly as a row without it, and existing servers and agents are unaffected on upgrade. Half a feature, stated as such: the gateway enforces this today, and an operator running the open-source gateway can set it through declarative config. Reaching it from AISIX Cloud needs the control plane to accept the new field — its validator rejects anything its spec does not list — so `cp-admin.yaml`, the typed model, the dashboard form and the i18n strings are the other half. Recorded on the roadmap as Cloud work rather than left implied by a merged gateway change. E2E over a real socket, because that is the only place a MATCHING allowlist can be shown to admit a caller — a oneshot request has no peer address, so unit tests can pin the refusal and nothing else. It asserts loopback is admitted, a foreign network is refused on both endpoints with their respective statuses, and that an unreachable MCP server publishes no tools. Against the pre-change binary the restricted agent serves with 200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Problem
arduino/setup-protoc@v3.0.0flakes intermittently withon
protoc-23.4-linux-x86_64.zipfrom the GitHub release CDN (objects.githubusercontent.com). It hits whichever of the four protoc-needing jobs (lint,schema drift,rust unit + coverage,build aisix (instrumented)) loses the lottery, failing unrelated PRs. Whenbuild aisix (instrumented)is the loser, the dependente2e+coverage >= 90%jobs are skipped, so a green-code PR shows red CI (this blocked #550 across three reruns).repo-tokenis already set on all four steps — but it only authenticates the GitHub API release lookup, not the asset download, so it does not prevent the 504.Fix
A local composite action
.github/actions/setup-protocthat downloads the same pinned protoc 23.4 with hard retries (curl --retry 5 --retry-all-errorsinside an outer 5× loop, 15 s backoff) and installs it under$HOME(nosudo), exposed to later steps via$GITHUB_PATH. Drop-in replacement at all four call sites (−12/+4inci.yml).protoc(e.g. the apt package on ubuntu-22.04 lacks proto3optional) silently changing build behavior.sudo— installs under$HOME/.local, pinsPROTOCfor prost/tonic build scripts.Validation
This PR modifies the workflow, so its own CI runs every job through the new install path — if they go green (incl.
build aisix (instrumented)→e2e), the fix is proven against the real flake.Why not apt / a retry-action
protobuf-compileris the simplest "never touch the CDN" fix but changes the protoc version (version-dependent build risk) — rejected to stay zero-risk.Integrity note: the asset is fetched over HTTPS from GitHub's official release CDN (TLS-authenticated), matching the integrity guarantee of the action it replaces (neither verifies a SHA-256). A pinned checksum can be added later if desired.