-
Notifications
You must be signed in to change notification settings - Fork 494
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
feat: Support self-hosted GitHub Enterprise servers #3748
base: master
Are you sure you want to change the base?
Conversation
It doesn't seem to work, unless I am missing something.
It seems like it is ignoring the host. |
It does seem to be using the correct access token though, just passing it to the wrong endpoint |
Doh! I forgot to set the URLs. Thanks for testing so quickly. PR updated. Please could you try again (I don't have a server to test against). |
4d5ad3d
to
b29a454
Compare
Thanks, it works now! Is it possible to have the auth token in a config file rather than polluting my environment? Not a big deal I am seeing a different error now but I think this one is my fault:
I verified that |
Great! Thanks for testing!
It's technically possible but I'd want to avoid storing secrets in chezmoi's config file. I'd like to think this through and get @halostatue's opinion before adding such a feature.
Maybe. This looks like the returned archive is not in the expected format, or maybe there's an HTTP redirect happening. Could you inspect the contents of the returned data? |
Yeah I'm just checking that now. When I download the archive through the browser and use |
If you run chezmoi with the |
Ah I see the issue. It's because I am doing this:
Let me try using |
Nah it doesn't work with |
Ah, I see, thank you. Can you embed the access token into the external URL, something like (untested):
|
Nah it doesn't work. GitHub doesn't allow basic auth, you have to pass in an |
OK, thank you again for testing. I suspect that the right solution is that chezmoi should add |
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.
LGTM.
It might be worthwhile to see about being able to use gh auth token
if that’s installed, but that’s a separate feature request. It would minimize the need for using CHEZMOI_GITHUB_ACCESS_TOKEN
etc.
@@ -1,12 +1,12 @@ | |||
# `gitHubLatestRelease` *owner-repo* | |||
# `gitHubLatestRelease` *host-owner-repo* |
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.
We may want to show this as *either *[host-]owner-repo*
or *owner-repo*|*host-howner-repo*
.
This applies to all github*
functions.
bindings](https://pkg.go.dev/github.com/google/go-github/v57/github#RepositoryRelease). | ||
|
||
Calls to `gitHubLatestRelease` are cached so calling `gitHubLatestRelease` with | ||
the same *owner-repo* will only result in one call to the GitHub API. | ||
the same *host-owner-repo* will only result in one call to the GitHub API. | ||
|
||
!!! example |
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.
Adding an example for host-owner-repo
in addition to owner-repo
would be good.
This applies to all github*
functions.
c8ae27a
to
7a68dfc
Compare
Fixes #3724.
@joshuaspence would you be able to test this?