-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ghe config option #559
Conversation
Signed-off-by: Sebastian Bezold <[email protected]>
Signed-off-by: Sebastian Bezold <[email protected]>
Signed-off-by: Sebastian Bezold <[email protected]>
44a3eda
to
485f3a8
Compare
operator.md
Outdated
export ALLSTAR_GHE_URL="https://my-ghe.example.com" | ||
export GH_HOST="my-ghe.example.com" # This is somehow used within a dependency and (as of now) is unclear how to set it programmatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GH_HOST
is used by the Scorecard dependency when setting up our clients to interact with GitHub. We don't currently have a way of setting this without using an environment variable. If this is a problem, I'm sure we could consider alternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, it is fine to just mention it in the docs. Thanks for the explanation 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks great! Just a couple nits.
.gitignore
Outdated
@@ -2,3 +2,4 @@ cmd/allstar/allstar | |||
*.pem | |||
.vscode | |||
allstar.ref | |||
.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline at end of file
pkg/ghclients/ghclients.go
Outdated
if i == 0 { | ||
tr, err = ghinstallationNewAppsTransport(ctr, operator.AppID, g.key) | ||
appTransport, _ := ghinstallationNewAppsTransport(ctr, operator.AppID, g.key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check error
pkg/ghclients/ghclients.go
Outdated
tr, err = ghinstallationNewAppsTransport(ctr, operator.AppID, g.key) | ||
appTransport, _ := ghinstallationNewAppsTransport(ctr, operator.AppID, g.key) | ||
// other than clien.WithEnterpriseUrls, setting the BaseUrl plainly, we need to ensure the /api/v3 ending | ||
appTransport.BaseURL = fullEnterpriseApiUrl(operator.GitHubEnterpriseUrl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap in if operator.GitHubEnterpriseUrl != "" {
pkg/ghclients/ghclients.go
Outdated
} else { | ||
tr, err = ghinstallationNew(ctr, operator.AppID, i, g.key) | ||
ghiTransport, _ := ghinstallationNew(ctr, operator.AppID, i, g.key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check error
Hi @spencerschrock and @jeffmendoza, sorry for the very late response. Have been on a longer private absence and was not able to work on this. |
Signed-off-by: Sebastian Bezold <[email protected]>
Signed-off-by: Sebastian Bezold <[email protected]>
Hi @jeffmendoza, are the fixes to your comments ok? Is there anything else, that I should adapt? |
Thanks for the reminder. Looks great. Thank you!! |
Description
This PR introduces support for self hosted GitHub Enterprise instances with URLs different than github.com.
With these changes, an environment variable
ALLSTAR_GHE_URL
can used to configure a custom GitHub URL.If set, all GitHub clients created are initialized using this URL.
Additional Info
For some reason, this setup only works if additionally to
ALLSTAR_GHE_URL
, you also defineGH_HOST
with the same host, just except the protocol (i.e:ALLSTAR_GHE_URL="https://my-ghe.example.com"
andGH_HOST="my-ghe.example.com"
).I have tested this PR on our own internal GitHub enterprise instance with
KEY_SECRET="direct"
and providing thePRIVATe_KEY
as plain-text.pem
Closes #552
Sebastian Bezold [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/impressum