Add support for GitHub app authentication

[risset]

Related issue: https://github.com/kubernetes/git-sync/issues/877

Currently the e2e::github_app_auth test should work locally, if the correct env vars are set. I've tested the feature that way before creating the PR. Ideally it would be possible to run it in the GHA workflow. One way I thought that might be feasible is if an app was created and installed to a particular (private) repo, and then its private key was stored as a git-sync repository secret.

Some information about GitHub app authentication: https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: risset / name: Liam Wyllie (5822b73d5b926462e7dd020ef2d12acbe807ce25)
• :white_check_mark: login: nan-yu / name: Nan Yu (ba2fe67a9743fe0295b18203bb9869464edebda8)

[k8s-ci-robot]

Welcome @risset!

It looks like this is your first PR to kubernetes/git-sync 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/git-sync has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: risset Once this PR has been reviewed and has the lgtm label, please ask for approval from thockin. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[nan-yu]

While setting up a Github app, I noticed instead of using App ID to get installation tokens, it also supports using the Client ID.

I'm wondering have you experimented with the client ID?

[risset]

While setting up a Github app, I noticed instead of using App ID to get installation tokens, it also supports using the Client ID.

I'm wondering have you experimented with the client ID?

I didn't know about this, thanks. It appears to be a recent feature: https://github.blog/changelog/2024-05-01-github-apps-can-now-use-the-client-id-to-fetch-installation-tokens/

So the App ID and Client ID can be used interchangeably in this case.

The application ID is not being deprecated at this time, nor are their plans to remove it. However, compatibility with future features will rely on use of the client ID, so updating is recommended.

Should we have both the app ID and client ID as parameters and accept either of them, or just replace app ID with client ID?

[nan-yu]

Should we have both the app ID and client ID as parameters and accept either of them, or just replace app ID with client ID?

I would vote for supporting both scenarios.

[nan-yu]

Hi @risset , any recent update on addressing the comments in this PR?

I'm working on Config Sync. If possible, we would like to leverage this feature in our next minor release in July.

[thockin]

Hi Nan,

Before we can merge this we need to make sure we can test it properly, which involves a private key. I have not had the time to play and see how private it really needs to be, or if maybe we can get it to test via GH actions.

If you have time, that's where we need to spend it.

On Mon, Jun 24, 2024, 9:38 PM Nan Yu @.***> wrote:

Hi @risset https://github.com/risset , any recent update on addressing the comments in this PR?

I'm working on Config Sync https://cloud.google.com/kubernetes-engine/enterprise/config-sync/docs/overview. If possible, we would like to leverage this feature in our next minor release in July.

— Reply to this email directly, view it on GitHub https://github.com/kubernetes/git-sync/pull/878#issuecomment-2187960417, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKWAVAJOQGHD6LXYS6QTZDZJDX5HAVCNFSM6AAAAABH6TO2F6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBXHE3DANBRG4 . You are receiving this because you were assigned.Message ID: @.***>

[risset]

Hi @risset , any recent update on addressing the comments in this PR?

Hi, I will be working on this some more over the next few days! I've been having some issues with running test_e2e.sh on my current (macOS) machine, but I'm able to test my changes on another Linux machine at least.

[nan-yu]

Hi Liam Wyllie , any recent update on addressing the comments in this PR?

Hi, I will be working on this some more over the next few days! I've been having some issues with running test_e2e.sh on my current (macOS) machine, but I'm able to test my changes on another Linux machine at least.

Thanks for the update. Did you run the e2e test with a public repo or a private repo? I followed the instruction with a private repo, and git-sync returned a repo not found error. Not sure if that works for you.

[risset]

Hi Liam Wyllie , any recent update on addressing the comments in this PR?

Hi, I will be working on this some more over the next few days! I've been having some issues with running test_e2e.sh on my current (macOS) machine, but I'm able to test my changes on another Linux machine at least.

Thanks for the update. Did you run the e2e test with a public repo or a private repo? I followed the instruction with a private repo, and git-sync returned a repo not found error. Not sure if that works for you.

I've been testing with a private repo. I'll double check the instructions and how I've set the app up etc. in case I missed something

[risset]

Hi Liam Wyllie , any recent update on addressing the comments in this PR?

Hi, I will be working on this some more over the next few days! I've been having some issues with running test_e2e.sh on my current (macOS) machine, but I'm able to test my changes on another Linux machine at least.

Thanks for the update. Did you run the e2e test with a public repo or a private repo? I followed the instruction with a private repo, and git-sync returned a repo not found error. Not sure if that works for you.

I've been testing with a private repo. I'll double check the instructions and how I've set the app up etc. in case I missed something

Does the app you created definitely have read-only or read-write access for Contents under Repository permissions, and is installed to the private repository itself (only select repositories) under Install App?

When I re-run the tests with either of those not true, I also get the repo not found error.

[nan-yu]

Hi Liam Wyllie , any recent update on addressing the comments in this PR?

Hi, I will be working on this some more over the next few days! I've been having some issues with running test_e2e.sh on my current (macOS) machine, but I'm able to test my changes on another Linux machine at least.

Thanks for the update. Did you run the e2e test with a public repo or a private repo? I followed the instruction with a private repo, and git-sync returned a repo not found error. Not sure if that works for you.

I've been testing with a private repo. I'll double check the instructions and how I've set the app up etc. in case I missed something

Does the app you created definitely have read-only or read-write access for Contents under Repository permissions, and is installed to the private repository itself (only select repositories) under Install App?

When I re-run the tests with either of those not true, I also get the repo not found error.

No worries. I think it was the issue with my testing account. I created a robot Github account. The account is flagged and requires reinstatement.

It works with my personal account.

[risset]

No worries. I think it was the issue with my testing account. I created a robot Github account. The account is flagged and requires reinstatement.

It works with my personal account.

Nice! Hopefully we can get this over the line. I'm still not quite sure how getting the tests to run properly via actions will work.

[nan-yu]

Regarding the e2e test for the github app integration, I created a robot GitHub account, a GitHub app, along with a private repository under the account. After that, I set up the required environment variables in GitHub Secret in my personal fork. All e2e test passed with changes in https://github.com/risset/git-sync/pull/1: https://github.com/nan-yu/git-sync/actions/runs/9947634472.

Can we use the robot account for testing, @thockin ?

[risset]

Have updated to address recent feedback !

Pardon my ignorance here but I'm still a bit unsure on how best to address your comment on the usage of io.ReadAll to read the HTTP response body. Is reading with io.Copy to a bytes.Buffer preferable here, or is there a more efficient approach to that?

Also I've added one, but not sure if you feel a metric is actually needed for this feature, it was just sloppiness from me having the askpass code fragment left in there before ...

[sdowell]

@risset @thockin Is this PR still being worked on?

[risset]

@risset @thockin Is this PR still being worked on?

Yeah I was just waiting for @thockin as he said he'd be away for a while. Also I think your review comments make sense but since @nan-yu wrote those parts of test_e2e.sh, I thought they might have some thoughts on it before I change those things. @nan-yu what do you think?

[thockin]

Yes, the ball is in my court to evaluate testing. I apologize, it has been a chaotic few weeks since I got back from holiday. I have not forgotten it.

[thockin]

Since this is a kubernetes repo, I'll have to see about making the kubernetes org own the app

[HorusTheSonOfOsiris]

Since this is a kubernetes repo, I'll have to see about making the kubernetes org own the app

Any plans on releasing this soon? This could help our team a lot.

[thockin]

I'm going to merge this and followup. I Still don't have CI for GH apps up, though.