Support GitHub App authentication

risset commented 6 months ago

Older issues on this topic: https://github.com/kubernetes/git-sync/issues/450 https://github.com/kubernetes/git-sync/issues/769

Hey, I wanted to revive this discussion again since I have a use case that would benefit from using a GitHub app for auth, and using an operator like that github-app-operator isn't ideal for me right now.

Would the following design potentially work?

allow github app private key, app ID and installation ID as parameters to git-sync
if github app auth is enabled, in refreshCreds, request a new token (can check if it's expired first)
- I think something similar to this should work: https://github.com/samirtahir91/github-app-operator/blob/0aca1da117933cad3075a55565eb422c0784f30c/internal/controller/githubapp_controller.go#L727
should then be able to authenticate over HTTPS with that token

CallAskPassURL is doing something a bit like this already, but doesnt give you much control over the request it makes.

I'm not too familar with git-sync internals yet but I have plenty of time to implement and try this out if maintainers think it might work

thockin commented 6 months ago

In this area, git-sync is not very complex. Something like you describe sounds fine. Ideally we would start with an e2e case, but it's not clear to me if there's an easy way to e2e reproduce the experience locally. Is there a trivial github app that we can use to at least manually run tests? Hard to support something I can't test at all :)

risset commented 6 months ago

Yeah, testing this doesn't seem straightforward :( I'm not sure of being able to use an existing public app for testing. When creating a new app, GitHub gives you a private key which running applications can use along with the app ID and installation ID (tied to a repo or organization) to authenticate as that particular app installation. So whoever is running the test would probably have to create a dummy app that's installed to the repo being synced and provide those three values to the test, which isn't great.

thockin commented 6 months ago

Can we automate or at least document clearly how I (an arbitrary person) could create my own "just for testing" app? Or that we could set up thru GH actions to run, but maybe an arbitrary person cant?

risset commented 6 months ago

I could create documentation for creating a testing app. Automation might be tricky since there's no GitHub API endpoints or the like for creating apps but it's worth looking into further

risset commented 6 months ago

Update from me on this, I've got the change working now and there's a couple things I'm unsure about before opening a PR (should have that done by the weekend)

Where should documentation for creating a github app for testing go? The docs directory seemed more user-facing
I've been testing my change by running a git-sync container in a similar way to the current github https e2e test, but with the addition of the new github app args which need to be supplied. Any ideas on how this can be integrated cleanly alongside the existing tests?

Thanks :)

thockin commented 6 months ago

Doc could go in "docs/dev/testing_github_app_auth.md" or something like that?

Tests: I am not sure I follow the question. I don't have a way to write an e2e test that doesn't always run, but that seems plausibly useful. If you write a PR which treats it like a normal e2e function, I will find a way to let you designate it as "skipped by default" or something

k8s-triage-robot commented 3 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue as fresh with /remove-lifecycle stale
Close this issue with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 months ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue as fresh with /remove-lifecycle rotten
Close this issue with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

kubernetes / git-sync

Support GitHub App authentication #877