Closed fbartho closed 2 months ago
My recommendation:
Change all the places in this general pattern:
- process.env["DANGER_GITHUB_API_BASE_URL"] || process.env["GITHUB_URL"] || "https://api.github.com"
+ process.env["DANGER_GITHUB_API_BASE_URL"] || "https://api.github.com"
- process.env["DANGER_GITHUB_API_BASE_URL"] || process.env["GITHUB_URL"] || undefined
+ process.env["DANGER_GITHUB_API_BASE_URL"] || "https://api.github.com"
Any objections? I'm happy to submit this change.
I've never had an enterprise account, so it was/is mostly guesswork - this does look like a breaking change though?
@orta While this is a paid account, it's not a "Github Enterprise" account issue. It is, however, when you want to host GitHub runners on your own machines which is very enterprise-y.
I think technically it's a breaking change, but it would only break some hypothetical environment that doesn't behave like Github-Public.
I guess I can't test what the Github Enterprise / self-hosted
experience behaves like.
When you're not on one of github-public's hosted runners you can't make github-api calls against https://github.com
so to me this is probably a safe change.
Agree
Today I ran into the same issue as @fbartho mentions in this thread. Is it ok for me to open a PR with the changes mentioned above to solve this issue? @orta
Describe the bug
We recently switched from Github-hosted Actions runners to Custom Runners because our CI minutes were getting too expensive, and hosting our own runners solved that problem.
But we ran into one last problem: some API calls were being sent to
https://github.com/<API_PATH>
-- but these API Paths are invalid when you use Octokit from outside of Github.Workaround: set
DANGER_GITHUB_API_BASE_URL: "https://api.github.com"
We verified that our environment variables had been the same in both locations, so my hypothesis is that when running in Github Infrastructure, you can make API calls against
github.com
instead of being required to useapi.github.com
Looking at danger-js' code, I find a handful of places where it's using `|| process.env["GITHUB_URL"]` as a fallback URL for `DANGER_GITHUB_API_BASE_URL` -- when I think it shouldn't?
``` 7 results - 5 files CHANGELOG.md: 1722 - Updated TypeScript and Jest dependencies - [@orta] 1723: - Add support for Github Enterprise via DANGER_GITHUB_API_BASE_URL env var - mashbourne 1724 source/platforms/github/customGitHubRequire.ts: 63 const containsBase = path.startsWith("http") 64: const baseUrl = process.env["DANGER_GITHUB_API_BASE_URL"] || process.env["GITHUB_URL"] || "https://api.github.com" 65 const URLPath = `repos/${repoSlug}/contents/${path}${refString}` source/platforms/github/GitHubAPI.ts: 47 48: const host = process.env["DANGER_GITHUB_API_BASE_URL"] || process.env["GITHUB_URL"] || undefined 49 const options: ConstructorParametersTo Reproduce Steps to reproduce the behavior:
Expected behavior DangerJS should work equally well in Github Actions Runners (on Github Infrastructure) and on Custom Runners.
Your Environment
Additional context Add any other context about the problem here.