Fix bug in github-app.ts #5039

AlexTugarev · 2021-08-02T10:16:14Z

First changes towards #4661

AlexTugarev · 2021-08-02T10:19:50Z

components/server/ee/src/prebuilds/github-app.ts

            const installationId = `${ctx.payload.installation.id}`;
-            const senderId = `${ctx.payload.sender.id}`;
-            const user = await this.userDB.findUserByIdentity({ authProviderId: this.env.githubAppAuthProviderId, authId: accountId });


accountId is the target account id, e.g. from github.com/gitpod-io. this are not valid coordinates for user accounts.

Interesting. I'd love to learn more about the differences between ctx.payload.installation.account.id and ctx.payload.sender.id.

From the docs you linked, I gather that ctx.payload.installation.account.id is the "account" on which the app is installed (e.g. can be an org, as you mentioned)

Who is ctx.payload.sender.id then? Is it the user that installed the app (e.g. a team member), or the user that triggered/caused the event? (e.g. a community contributor opening a new PR)

See the documentation URL at L.90.

jankeromnes · 2021-08-02T20:12:28Z

Taking a look! Many thanks. 👍

/werft run

👍 started the job as gitpod-build-at-github-app.1

jankeromnes

Changes look good to me, thanks!

However, I tried the following test:

Connect my GitHub App to this PR
Add a Project for https://github.com/jankeromnes/oops.git
Rename repository to "oops2"
... the cloneUrl in the DB never got updated:

mysql> select name, cloneUrl from d_b_project;
+------+-----------------------------------------+
| name | cloneUrl                                |
+------+-----------------------------------------+
| oops | https://github.com/jankeromnes/oops.git |
+------+-----------------------------------------+
1 row in set (0.01 sec)

Did the rename listener work for you?

components/server/ee/src/prebuilds/github-app.ts

jankeromnes · 2021-08-02T20:04:18Z

components/server/ee/src/prebuilds/github-app.ts

            const installationId = `${ctx.payload.installation.id}`;
-            const senderId = `${ctx.payload.sender.id}`;
-            const user = await this.userDB.findUserByIdentity({ authProviderId: this.env.githubAppAuthProviderId, authId: accountId });


Interesting. I'd love to learn more about the differences between ctx.payload.installation.account.id and ctx.payload.sender.id.

From the docs you linked, I gather that ctx.payload.installation.account.id is the "account" on which the app is installed (e.g. can be an org, as you mentioned)

Who is ctx.payload.sender.id then? Is it the user that installed the app (e.g. a team member), or the user that triggered/caused the event? (e.g. a community contributor opening a new PR)

jankeromnes · 2021-08-02T20:10:28Z

components/server/ee/src/prebuilds/github-app.ts

        });
        app.on('installation.deleted', async ctx => {
            const installationId = `${ctx.payload.installation.id}`;
            await this.appInstallationDB.recordUninstallation("github", 'platform', installationId);
        });

+        app.on('repository.renamed', async ctx => {


Interesting, where is this event type documented? I couldn't find it anywhere.

Also, that's the repository event with renamed action. See the documentation URL above.

jankeromnes · 2021-08-02T20:10:51Z

components/server/ee/src/prebuilds/github-app.ts

+            if (!installation) {
+                return;
+            }


Q: Can this actually happen?

At least the typings suggest it by declaring it optional.
cf. https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#repository

jankeromnes · 2021-08-02T21:10:54Z

Hmm, maybe something is wrong with my GitHub App setup:

mysql> select appInstallationId from d_b_project;
+-------------------+
| appInstallationId |
+-------------------+
| 18634854          |
| 18639799          |
+-------------------+
2 rows in set (0.00 sec)

mysql> select * from d_b_app_installation;
Empty set (0.00 sec)

AlexTugarev · 2021-08-03T07:22:11Z

@jankeromnes, I just double-checked with the webhook events and they look good. Did you set the webhook URL properly? You can check the settings of the GH App for the event delivery status, maybe they are not 200 OK?

This is a short version of what's received, and it was effective:

{
    "action": "renamed",
    "changes": { "repository": { "name": { "from": "test-repo" } } },
    "repository": {
        "name": "test-repo-1",
        "full_name": "cool-test-org/test-repo-1",
        "owner": {
            "login": "cool-test-org", ...
        }, ...
    },
    "organization": {
        "login": "cool-test-org", ...
    },
    "sender": {
        "login": "AlexTugarev", ...
    },
    "installation": {
        "id": 18635259, ...
    }
}

svenefftinge

lgtm

roboquat · 2021-08-03T07:34:42Z

LGTM label has been added.

Git tree hash: e1c13db6df568b815cc6524306a8dc680353f2aa

roboquat · 2021-08-03T07:35:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: svenefftinge

Associated issue: #4661

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~components/server/OWNERS~~ [svenefftinge]

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

jankeromnes · 2021-08-03T07:49:57Z

I just double-checked with the webhook events and they look good. Did you set the webhook URL properly?

Hmm, I thought I did. I had set it to https://at-github-app.staging.gitpod-dev.com/api/apps/github:

You can check the settings of the GH App for the event delivery status, maybe they are not 200 OK?

Ah, very cool, I didn't know about that.

Checking the event log, it seems that:

The first event ("repository added") failed. This is surprising to me, because the project was successfully added.
The second event ("pushed to repository") succeeded.

There was no event about the rename, apparently. Maybe that's an additional option that needs to be enabled in the GitHub App somewhere? 🤔

Anyway, glad this is now merged! 🚀

AlexTugarev · 2021-08-03T07:56:53Z

@jankeromnes, you've created the GH App after the gitpod-staging-com App? Just realized it's not using this event, see the event and permissions page where you'll find the Repository event. That's like missing. Also, I'm checking with the production app.

jankeromnes · 2021-08-03T07:58:11Z

@AlexTugarev Ah yes, I've duplicated permissions from https://github.com/organizations/gitpod-io/settings/apps/gitpod-staging-com/permissions. That would explain it!

EDIT: Which additional permissions do I need to add? Also, we should probably add them to the staging app too, right? Maybe to the production app as well?

AlexTugarev added 2 commits August 2, 2021 10:13

[github-app] fix installation.created handler

0eb10d0

[github-app] handle repository.renamed

915c36c

roboquat requested a review from csweichel August 2, 2021 10:16

roboquat added the size/S label Aug 2, 2021

AlexTugarev requested a review from svenefftinge August 2, 2021 10:16

AlexTugarev changed the title ~~at/github-app~~ Fix bug in github-app.ts Aug 2, 2021

AlexTugarev commented Aug 2, 2021

View reviewed changes

AlexTugarev requested a review from jankeromnes August 2, 2021 14:22

jankeromnes reviewed Aug 2, 2021

View reviewed changes

svenefftinge approved these changes Aug 3, 2021

View reviewed changes

roboquat assigned svenefftinge Aug 3, 2021

roboquat added the lgtm label Aug 3, 2021

roboquat added the approved label Aug 3, 2021

roboquat merged commit 7f0908b into main Aug 3, 2021

roboquat deleted the at/github-app branch August 3, 2021 07:36

gtsiolis added the 🧑‍🚀 crew: teams and projects label Aug 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug in github-app.ts #5039

Fix bug in github-app.ts #5039

AlexTugarev commented Aug 2, 2021

AlexTugarev Aug 2, 2021

jankeromnes Aug 2, 2021

AlexTugarev Aug 3, 2021

jankeromnes commented Aug 2, 2021 •

edited by werft-gitpod-dev-com bot

jankeromnes left a comment

jankeromnes Aug 2, 2021

jankeromnes Aug 2, 2021

AlexTugarev Aug 3, 2021

jankeromnes Aug 2, 2021

AlexTugarev Aug 3, 2021

jankeromnes commented Aug 2, 2021

AlexTugarev commented Aug 3, 2021

svenefftinge left a comment

roboquat commented Aug 3, 2021

roboquat commented Aug 3, 2021

jankeromnes commented Aug 3, 2021 •

edited by werft-gitpod-dev-com bot

AlexTugarev commented Aug 3, 2021 •

edited

jankeromnes commented Aug 3, 2021 •

edited

Fix bug in github-app.ts #5039

Fix bug in github-app.ts #5039

Conversation

AlexTugarev commented Aug 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jankeromnes commented Aug 2, 2021 • edited by werft-gitpod-dev-com bot

jankeromnes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jankeromnes commented Aug 2, 2021

AlexTugarev commented Aug 3, 2021

svenefftinge left a comment

Choose a reason for hiding this comment

roboquat commented Aug 3, 2021

roboquat commented Aug 3, 2021

jankeromnes commented Aug 3, 2021 • edited by werft-gitpod-dev-com bot

AlexTugarev commented Aug 3, 2021 • edited

jankeromnes commented Aug 3, 2021 • edited

jankeromnes commented Aug 2, 2021 •

edited by werft-gitpod-dev-com bot

jankeromnes commented Aug 3, 2021 •

edited by werft-gitpod-dev-com bot

AlexTugarev commented Aug 3, 2021 •

edited

jankeromnes commented Aug 3, 2021 •

edited