akuity
Promotion / create PR feature for GitHub Enterprise server
Checklist
- [x] I've searched the issue queue to verify this is not a duplicate feature request.
- [ ] I've pasted the output of
kargo version, if applicable. - [ ] I've pasted logs, if applicable.
Proposed Feature
With 0.3.0 we can now create PR as part of the promotion. This is the only remaining feature that blocks us to onboard Kargo.
Motivation
Enterprise corps use Github-Enterprise installations ;)
Suggested Implementation
Because of the similarities of Github- / Github Enterprise-APIs I hope its fairly simple to support also Github Enterprise servers.
Hi @MarkusNeuron!
@jessesuen designed the PR support so that it should be relatively easy to add support for additional git providers. It was always our intention to start with one and add more over time -- or recruit community assistance to add more over time.
If memory serves, the main difference between GitHub and GitHub Enterprise is in how the client is constructed. i.e. In the case of GitHub, there's no need to provide a base URL, but for GitHub Enterprise, one must. Again, if memory serves, once that's done, everything else mostly works the same. So, just as you guess, I think most of what @jessesuen already did for GitHub could probably be re-used.
The "hard" part in this case is figuring out one or both of:
- How to infer from a repo URL that the repo is hosted in GitHub Enterprise.
- Assuming that cannot be inferred in all cases, how can users provide a hint (through a new CRD attribute, probably) that the repo is hosted in GitHub Enterprise?
@jessesuen idk if you might want to add any insight into this?
Yes, I think support for this might be as simple as introducing logic to infer that the hostname is enterprise github, then calling go-github with the WithEnterpriseURLs option:
NewClient(httpClient).WithEnterpriseURLs(baseURL, uploadURL)
How I see the UX for this:
-
Today, if we detect if the word "github" is in the hostname, we assume that it is GitHub. I trust that this will always be a safe assumption. However, currently we don't properly configure the client to use the new hostname. So the only thing left to do is: if "github" is in the hostname, but the hostname is not exactly "github.com", then we must call
WithEnterpriseURLs(baseURL, uploadURL). -
Even with the above, there would be a case we wouldn't handle, which is: a user's github enterprise server might have a hostname like:
git.mycompany.com. In this case, we couldn't/wouldn't assume it is github, and so we would need a system-wide setting to explicitly indicategit.mycompany.comis of type GitHub (and not BitBucket, etc...).
@MarkusNeuron option 1 should be quite easy to implement with a few lines of code, but not sure if hostname guessing would work for your use case, or if you would need an explicit system setting to indictate it.
how can users provide a hint (through a new CRD attribute, probably)
I think we were mostly saying the same thing. The difference in my proposal is for the hint to be done at a system level, as opposed to a CRD-level attribute.
The difference in my proposal is for the hint to be done at a system level, as opposed to a CRD-level attribute.
Agree. In hindsight, the CRD attribute suggestion suggestion didn't make sense.
Hi @krancour and @jessesuen, first let me thank you to your great response times and cool suggestions. Mid- / Log-term I see now way around to manually configure the git provider & corresponding URL in some way ( CRD, annotations, ...) to flexible support different git providers.
As a quick hack the proposed integration...
but the hostname is not exactly "github.com", then we must call WithEnterpriseURLs(baseURL, uploadURL)
... would close our gap. Anyhow thanks for consideration.
This should have been closed by #2158 which addressed the need to set baseURL for GtiHub Enterprise clients at the same time that the same problem was being solved for self-hosted GitLab.