cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

`isEnterprise` returns `true` incorrectly on `github.localhost` #48

Closed JasonEtco closed 2 years ago

JasonEtco commented 2 years ago

Hello! I have an admittedly niche problem - I'm working on a CLI extension against github.localhost, which means setting GH_HOST=github.localhost for each command. Calling isEnterprise returns true in that case, because it is not github.com:

https://github.com/cli/go-gh/blob/9dbbfe25d9431c8044018aeb7f101c12fdcc1394/internal/config/config.go#L110-L112

That results in an incorrect (and un-fixable) URL here:

https://github.com/cli/go-gh/blob/9dbbfe25d9431c8044018aeb7f101c12fdcc1394/internal/api/rest_client.go#L112-L117

As far as I can tell, there's no way for me to say "use GH_HOST, but also use api.<GH_HOST> instead of <GH_HOST>/api/v3". Is that correct, or is there a patch needed here? Happy to open a PR adding an exception for github.localhost if that makes sense, or some more consistent way of setting the full URL?


FWIW, I did think that maybe have GH_HOST=http://api.github.localhost would work, but that gives:

Post "https://http//api.github.localhost/api/v3/repos/:owner/:repo/<REDACTED>"
mislav commented 2 years ago

@JasonEtco Thank you for reporting! This seems to be a bug.

@samcoe What do you think about porting hostname normalization logic over from CLI? https://github.com/cli/cli/blob/4c237708d878008e85e8190d310dc1826b463ca9/internal/ghinstance/host.go#L20-L22

samcoe commented 2 years ago

@JasonEtco @mislav Yup, we should port over the localhost handling in the hostname normalization logic, it was just an oversite that it isn't already there. It if it isn't super urgent to get this fixed, to avoid conflicts, perhaps we can add it after #44 and #45 land since those PR's change the location of normalization code.

JasonEtco commented 2 years ago

Ah great, thanks for confirming its a bug y'all! This is not super urgent, I can hack around it locally for now.