Migrate Prow GitHub authentication from Personal Access Token to a GitHub App

[ccojocar]

What would you like to be added:

Prow is currently using a Personal Access Token to authenticate with GitHub API. In the meantime, GitHub has revamped the API authentication mechanism and introduced GitHub Apps, which will replace soon the Personal Access Token and OAuth tokens.

Migrate prow from a Personal Access Token to a GitHub App.

Why is this needed:

GitHub Apps provide some advantages over Personal Access Tokes which are listed here:
https://developer.github.com/apps/migrating-oauth-apps-to-github-apps/

cc @rawlingsj @wbrefvem

/kind feature
/area prow

[stevekuznetsov]

Not sure it's required for V4 -- we're successfully using OAuth tokens for the V4 API access we do today. I think we've talked about this in the past but it was unclear what the effort involved would be and what the downsides might be for the project.

/cc @fejta @spiffxp

[michaelneale]

certainly for access to checks api (perhaps not v4). Github are very keen on shifting to app vs PAT

[ccojocar]

I updated the description of the issue with the official reasons mentioned by GitHub in order to avoid any controversy.

[BenTheElder]

cc @cjwagner

[jlewi]

I think an advantage of GitHub apps over using PAT is that GitHub apps are owned by the organization. I think this will make them much easier to administer than bot accounts which requires sharing access to a GitHub account.

I dug into this an it looks to me like the way it works is that GitHub apps use private certs to generate JWTs which are then used to authenticate to the GitHub APIs: e.g. see the examples
https://github.com/github-developer/using-the-github-api-in-your-app/blob/master/server.rb#L104

So I think the primary work would be updating your GitHub client to generate JWTs

test-infra/ghclient/core.go

Line 49 in e591cdf

func NewClient(token string, dryRun bool) *Client {

Some googling turned up the following library for generating JWTs in go:
https://github.com/dgrijalva/jwt-go

[fejta]

Here is how we can update tackle to help make this easier: https://developer.github.com/apps/building-github-apps/creating-github-apps-from-a-manifest/

Note that this change would have some downsides. We could not, for example, react to a comment/issue

[stevekuznetsov]

It was noted that this approach will not give us the same access to the API as we have with PAT -- some APIs are PAT-only. Not sure if any of those are ones we use, though, or what the set is, even.

[michaelneale]

I have heard that github want to replace PAT at some point so any gaps in api coverage are likely temporary anyway.

[stevekuznetsov]

Sure, but:

• PAT is not going away in the near future
• we cannot migrate until we have all of our API usage supported

Not a blocker, just more things to think about when this work is undertaken. Agreed that the official stance from GitHub is that we should in a perfect world move away from PAT.

[spiffxp]

We would also get way, way more tokens if we interacted as an app against high-volume orgs like kubernetes.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[michaelneale]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[petr-muller]

/remove-lifecycle rotten

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

[alvaroaleman]

/remove-lifecycle rotten

This is currently missing the Tide bits, the rest works already. I'll try to get the tide bits done in the near future, I have some work on that locally already.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[spiffxp]

maybe ref: #20713 (if Actions integration implies interacting with Checks)

[k8s-triage-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

[alvaroaleman]

Tide gh apps support was finished in #23036, we rolled it out in Openshift today and it appears to be working fine.

That means the only thing left here is to update our documentation and samples.

[BenTheElder]

Was that the last thing to being able to migrate prow fully? Should we start looking at moving prow.k8s.io as well?

[alvaroaleman]

Was that the last thing to being able to migrate prow fully?

Yes

Should we start looking at moving prow.k8s.io as well?

I think that would be great :) I am not sure if there is an immediate practical benefit to that, though. It is primarily useful to simplify onboarding orgs as the webhook and hmac secret are configured on the app instead of on the org and to alleviate token exhaustion issues (which afaik do not exist for prow.k8s.io).

One thing to note is that if a github apps needs to be installed in more than one GH org, everyone will be able to install it[1]. While not great this is IMHO ok, as Prow won't do anything for events in orgs or repos it has no config for.

[chaodaiG]

Tide gh apps support was finished in #23036, we rolled it out in Openshift today and it appears to be working fine.

I assume that the gh apps user name is different from previous prow bot account, so during the transition period prow might do something inconsistent, for example it will not find previous existing bot comment and comment something new.

Is there any other side effect that you have observed? In another word, if we want to migrate one of our prow instance over, what should we communicate with our users?

[alvaroaleman]

I assume that the gh apps user name is different from previous prow bot account, so during the transition period prow might do something inconsistent, for example it will not find previous existing bot comment and comment something new.

Yes, that is correct, e.G. approve will leave a stale comment behind on existing PRs.

Is there any other side effect that you have observed? In another word, if we want to migrate one of our prow instance over, what should we communicate with our users?

No, we actually did this relatively silently (and gradually through the --github-enabled/disabled-org/repo settings as we are the first user and it took some time until all the org owners installed the app).