mirror of
https://github.com/discourse/discourse.git
synced 2026-08-09 21:45:25 +08:00
GitHub oneboxes and the discourse-github plugin talked to GitHub's REST
and
GraphQL API with no rate-limit awareness. On busy instances this
exhausted
GitHub's limits (60 requests/hour unauthenticated, 5000 authenticated),
and
because there was no backoff every render kept hitting GitHub and
re-failing
-- which GitHub's docs warn can get an integration banned. The recently
added PR-status onebox multiplied the number of calls and made it far
worse.
GitHub access was also fragmented: the core onebox engines used OpenURI,
the
discourse-github plugin used Octokit, and the discourse-ai bot tools
used
FinalDestination::HTTP -- three HTTP stacks, three tokens, and
inconsistent
(or entirely missing) error and rate-limit handling.
This introduces a single client, Discourse::GithubApi, that every GitHub
data-API request now flows through. It is built on Faraday with the
SSRF-safe
FinalDestination adapter and:
- authenticates per token (Bearer) and returns plain string-keyed Hashes
(get/post) or raw bodies (raw_get) -- one response shape, no
Octokit/Sawyer
- only ever sends the access token to api.github.com and
raw.githubusercontent.com, rejecting any other absolute URL, so a
user-derived path can never leak a token to an arbitrary host
- backs off on rate limits both reactively (403/429) and proactively
(when
X-RateLimit-Remaining hits 0), honouring Retry-After /
X-RateLimit-Reset,
via a shared Redis flag (GithubRateLimit) keyed per token so each
token's
budget and the shared unauthenticated/IP budget back off independently
- short-circuits while backing off without ever sleeping, so onebox
rendering
and post baking degrade to a plain link instead of blocking a request
- caches ETags and sends If-None-Match, so unchanged resources return
304s
that do not count against the rate limit
Every caller was moved onto it:
- the 6 core GitHub onebox engines, via a slimmed
Onebox::Mixins::GithubApi
adapter that keeps their public methods and translates client errors
back
to the OpenURI::HTTPError vocabulary they already rescue (engines
unchanged)
- the github_blob raw.githubusercontent.com fetch
- the discourse-github plugin (badges, linkback, permalinks, token
validator),
which no longer uses the octokit and sawyer gems (they stay in the
Gemfile for
the discourse-code-review official plugin, which still depends on them)
- the discourse-ai bot's GitHub tools (search code, diff, file content,
search files)
Also adds a GithubOneboxBackoff admin problem check that surfaces while
one of
the onebox token identities is backing off -- scoped to the tokens
resolved by
Onebox::GithubAccess (each configured github_onebox_access_tokens entry
plus the
unauthenticated client) so a backoff on the AI bot or linkback token is
not
misattributed to onebox. Its message points admins at the relevant
setting with
the {{setting:...}} link marker, which problem-check messages now expand
too.
Onebox token resolution is centralised in Onebox::GithubAccess, and the
onebox
cache TTL for transient GitHub failures is shortened so they recover
quickly.
GitHub OAuth login, theme git-clone, the inbound webhook, and the
Oneboxer
FinalDestination URL-resolution special-cases for github.com are
intentionally
out of scope -- they are different concerns, not the rate-limited data
API.
121 lines
4.4 KiB
Ruby
Vendored
121 lines
4.4 KiB
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
describe Jobs::ReplaceGithubNonPermalinks do
|
|
let(:job) { described_class.new }
|
|
let(:github_url) do
|
|
"https://github.com/test/onebox/blob/master/lib/onebox/engine/github_blob_onebox.rb"
|
|
end
|
|
let(:github_permanent_url) do
|
|
"https://github.com/test/onebox/blob/815ea9c0a8ffebe7bd7fcd34c10ff28c7a6b6974/lib/onebox/engine/github_blob_onebox.rb"
|
|
end
|
|
let(:github_url2) { "https://github.com/test/discourse/blob/master/app/models/tag.rb#L1-L3" }
|
|
let(:github_permanent_url2) do
|
|
"https://github.com/test/discourse/blob/7e4edcfae8a3c0e664b836ee7c5f28b47853a2f8/app/models/tag.rb#L1-L3"
|
|
end
|
|
let(:broken_github_url) do
|
|
"https://github.com/test/oneblob/blob/master/lib/onebox/engine/nonexistent.rb"
|
|
end
|
|
let(:github_response_body) { { sha: "815ea9c0a8ffebe7bd7fcd34c10ff28c7a6b6974", commit: {} } }
|
|
let(:github_response_body2) { { sha: "7e4edcfae8a3c0e664b836ee7c5f28b47853a2f8", commit: {} } }
|
|
|
|
before do
|
|
enable_current_plugin
|
|
|
|
stub_request(:get, "https://api.github.com/repos/test/onebox/commits/master").to_return(
|
|
status: 200,
|
|
body: github_response_body.to_json,
|
|
headers: {
|
|
"Content-Type" => "application/json",
|
|
},
|
|
)
|
|
stub_request(
|
|
:get,
|
|
"https://api.github.com/repos/test/onebox/commits/815ea9c0a8ffebe7bd7fcd34c10ff28c7a6b6974",
|
|
).to_return(
|
|
status: 200,
|
|
body: github_response_body.to_json,
|
|
headers: {
|
|
"Content-Type" => "application/json",
|
|
},
|
|
)
|
|
stub_request(:get, "https://api.github.com/repos/test/oneblob/commits/master").to_return(
|
|
status: 404,
|
|
)
|
|
stub_request(:get, "https://api.github.com/repos/test/discourse/commits/master").to_return(
|
|
status: 200,
|
|
body: github_response_body2.to_json,
|
|
headers: {
|
|
"Content-Type" => "application/json",
|
|
},
|
|
)
|
|
end
|
|
|
|
describe "#execute" do
|
|
before do
|
|
Jobs.run_immediately!
|
|
SiteSetting.github_permalinks_enabled = true
|
|
end
|
|
|
|
it "replaces link with permanent link" do
|
|
stub_request(:head, github_permanent_url).to_return(status: 200, body: "", headers: {})
|
|
stub_request(
|
|
:get,
|
|
"https://raw.githubusercontent.com/test/onebox/815ea9c0a8ffebe7bd7fcd34c10ff28c7a6b6974/lib/onebox/engine/github_blob_onebox.rb",
|
|
).to_return(status: 200, body: "", headers: {})
|
|
|
|
post = Fabricate(:post, raw: github_url)
|
|
job.execute(post_id: post.id)
|
|
post.reload
|
|
|
|
expect(post.raw).to eq(github_permanent_url)
|
|
end
|
|
|
|
it "doesn't replace the link if it's already permanent" do
|
|
post = Fabricate(:post, raw: github_permanent_url)
|
|
job.execute(post_id: post.id)
|
|
post.reload
|
|
|
|
expect(post.raw).to eq(github_permanent_url)
|
|
end
|
|
|
|
it "doesn't change the post if link is broken" do
|
|
post = Fabricate(:post, raw: broken_github_url)
|
|
job.execute(post_id: post.id)
|
|
post.reload
|
|
|
|
expect(post.raw).to eq(broken_github_url)
|
|
end
|
|
|
|
it "works with multiple github urls in the post" do
|
|
stub_request(:get, github_permanent_url).to_return(status: 200, body: "")
|
|
stub_request(:get, github_permanent_url2.gsub(/#.+$/, "")).to_return(status: 200, body: "")
|
|
post = Fabricate(:post, raw: "#{github_url} #{github_url2} https://github.com")
|
|
job.execute(post_id: post.id)
|
|
post.reload
|
|
|
|
updated_post = "#{github_permanent_url} #{github_permanent_url2} https://github.com"
|
|
expect(post.raw).to eq(updated_post)
|
|
end
|
|
end
|
|
|
|
describe "#excluded?" do
|
|
before do
|
|
SiteSetting.github_permalinks_exclude =
|
|
"README.md|discourse/discourse/directory/file.rb|discourse/onebox/docs/*|discourse/anotherRepo/*|someUser/*"
|
|
end
|
|
|
|
it "returns true when it should be excluded" do
|
|
expect(job.excluded?("discourse", "discourse", "README.md")).to be true
|
|
expect(job.excluded?("discourse", "discourse", "directory/file.rb")).to be true
|
|
expect(job.excluded?("discourse", "onebox", "docs/file.rb")).to be true
|
|
expect(job.excluded?("discourse", "anotherRepo", "directory/file.rb")).to be true
|
|
expect(job.excluded?("someUser", "someRepo", "file.rb")).to be true
|
|
end
|
|
|
|
it "return false when url should be replaced" do
|
|
expect(job.excluded?("discourse", "discourse", "directory/file2.rb")).to be false
|
|
expect(job.excluded?("discourse", "onebox", "directory/file.rb")).to be false
|
|
expect(job.excluded?("discourse", "discourse", "directory/included.rb")).to be false
|
|
end
|
|
end
|
|
end
|