Skip to content
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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

twpayne
Copy link
Owner

@twpayne twpayne commented May 2, 2024

Fixes #3724.

@joshuaspence would you be able to test this?

@twpayne twpayne requested a review from halostatue May 2, 2024 20:40
@twpayne twpayne marked this pull request as ready for review May 2, 2024 21:17
@joshuaspence
Copy link

It doesn't seem to work, unless I am missing something.

$ chezmoi --version
chezmoi version dev, commit 95f58622290fda3494bf829c7cb5c3a6ea2b7c29, built at 2024-05-02T21:17:02Z

$ chezmoi execute-template '{{ gitHubLatestRelease "REDACTED_HOST/REDACTED_OWNER/REDACTED_REPO" }}'
chezmoi: template: arg1:1:3: executing "arg1" at <gitHubLatestRelease "REDACTED_HOST/REDACTED_OWNER/REDACTED_REPO">: error calling gitHubLatestRelease: GET https://api.github.com/repos/REDACTED_OWNER/REDACTED_REPO/releases/latest: 401 Bad credentials []

It seems like it is ignoring the host.

@joshuaspence
Copy link

It does seem to be using the correct access token though, just passing it to the wrong endpoint

@twpayne
Copy link
Owner Author

twpayne commented May 2, 2024

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).

@twpayne twpayne force-pushed the fix-3724 branch 2 times, most recently from 4d5ad3d to b29a454 Compare May 2, 2024 23:17
@joshuaspence
Copy link

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:

± ~/workspace/chezmoi/chezmoi diff
chezmoi: gzip: invalid header

I verified that chezmoi execute-template '{{ gitHubLatestRelease "REDACTED_HOST/REDACTED_OWNER/REDACTED_REPO" }}' works

@twpayne
Copy link
Owner Author

twpayne commented May 2, 2024

Thanks, it works now!

Great! Thanks for testing!

Is it possible to have the auth token in a config file rather than polluting my environment? Not a big deal

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.

I am seeing a different error now but I think this one is my fault:

± ~/workspace/chezmoi/chezmoi diff
chezmoi: gzip: invalid header

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?

@joshuaspence
Copy link

Yeah I'm just checking that now. When I download the archive through the browser and use file on it, it shows gzip compressed data, max compression, original size modulo 2^32 87956992. Trying to work out if chezmoi is seeing different data or if there is something else going on

@twpayne
Copy link
Owner Author

twpayne commented May 2, 2024

If you run chezmoi with the --debug flag I think it will show you the first few bytes of each HTTP download.

@joshuaspence
Copy link

Ah I see the issue. It's because I am doing this:

url: '{{ (gitHubLatestRelease "REDACTED").HTMLURL | replace "/releases/tag/" "/releases/download/" }}/REDACTED_{{ .chezmoi.os }}_{{ .chezmoi.arch | replace "amd64" "x86_64" }}.tar.gz'

Let me try using gitHubLatestReleaseAssetURL

@joshuaspence
Copy link

Nah it doesn't work with gitHubLatestReleaseAssetURL. I guess because whatever is processing externals is just using a generic HTTP client, which won't be injecting the GitHub auth token

@twpayne
Copy link
Owner Author

twpayne commented May 2, 2024

Ah, I see, thank you.

Can you embed the access token into the external URL, something like (untested):

["my/external"]
    url = "https://git.example.com:{{ env "CHEZMOI_GIT_EXAMPLE_COM_ACCESS_TOKEN" }}/path/to/archive"

@joshuaspence
Copy link

Nah it doesn't work. GitHub doesn't allow basic auth, you have to pass in an Authorization: Bearer xxxxxxx header instead

@twpayne
Copy link
Owner Author

twpayne commented May 3, 2024

OK, thank you again for testing.

I suspect that the right solution is that chezmoi should add Authentication: Bearer $CHEZMOI_GIT_EXAMPLE_COM_ACCESS_TOKEN headers to all requests to git.example.com, but this will require some thought and refactoring.

@twpayne twpayne marked this pull request as draft May 3, 2024 00:26
Copy link
Collaborator

@halostatue halostatue left a 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*
Copy link
Collaborator

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
Copy link
Collaborator

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.

@twpayne twpayne force-pushed the master branch 2 times, most recently from c8ae27a to 7a68dfc Compare August 5, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support self-hosted GitHub Enterprise
3 participants