[projects] enable webhook registration for bitbucket.org and self-managed gitlab #7422

AlexTugarev · 2022-01-03T14:47:37Z

Fixes registration of webhooks for projects hosted on bitbucket.org and self-managed gitlab.

gtsiolis · 2022-01-03T17:27:45Z

Looking at this now! 👀

gtsiolis

Thanks for picking this up, @AlexTugarev! Works as expected, this makes everything one step closer to having a consistent product behavior across GitLab and Bucket that require the webshook! 🔮

🍊 🍊 🍊 🍊

Opened some follow-up issues that can be considered out of the scope of this PR: #7424, #7425, #7426. Feedback is welcome!

🍋 🍋 🍋 🍋

Approving to unblock merging but holding in case we need someone from the team to take a closer look at the code changes.

/hold

gtsiolis · 2022-01-03T18:25:47Z

components/server/src/projects/projects-service.ts

@@ -115,7 +115,7 @@ export class ProjectsService {
    protected async onDidCreateProject(project: Project, installer: User) {
        let { userId, teamId, cloneUrl } = project;
        const parsedUrl = RepoURL.parseRepoUrl(project.cloneUrl);
-        if ("gitlab.com" === parsedUrl?.host) {
+        if ("gitlab.com" === parsedUrl?.host || "bitbucket.org" === parsedUrl?.host) {


issue(non-blocking): Prebuilds are still not enabled for the default branch for GitLab and Bitbucket repositories but certainly out of the scope of this PR. ➿

That's interesting. Is this already planned/discussed/tracked somewhere in a GH issue?

@AlexTugarev I could be missing something but I've added #7424 to continue the discussion about this issue there. Up for closing it if the issue is not accurate or valid. 🏀

roboquat · 2022-01-03T18:27:05Z

LGTM label has been added.

Git tree hash: cc0187c12c0ccb3fb53f4470a56675673090ab6e

gtsiolis · 2022-01-03T18:28:26Z

Also, a good follow up step after this could be to consider removing the webhook (#5517) when the project is removed from the dashboard, right? What do you think? Cc @jldec

jldec · 2022-01-03T20:29:51Z

@gtsiolis enabling prebuilds on all branches on Bitbucket should be part of fixing #7367 see comment

Keeping in mind that there are overhead costs to creating and tracking new issues, let's avoid spawning followup issues to followup issues, when asking for clarification in the issue at hand would suffice.

(similar feedback applies to self-hosted GitLab and #7416)

gtsiolis · 2022-01-03T21:03:17Z

... enabling prebuilds on all branches on Bitbucket should be part of fixing #7367

@jldec I agree but not entirely sure if there was a background decision for not enabling prebuilds for all branches by default, see relevant docs:

github:
  prebuilds:
    # enable for the default branch (defaults to true)
    master: true
    # enable for all branches in this repo (defaults to false)
    branches: false
    # enable for pull requests coming from this repo (defaults to true)
    pullRequests: true
    # enable for pull requests coming from forks (defaults to false)
    pullRequestsFromForks: false
    # add a check to pull requests (defaults to true)
    addCheck: true
    # add a "Review in Gitpod" button as a comment to pull requests (defaults to false)
    addComment: false
    # add a "Review in Gitpod" button to the pull request's description (defaults to false)
    addBadge: false

Also, I'm probably wrong here but this was most probably an efficient solution to 🅰️ avoid creating prebuilds for all branches while users were not using the product for other branches and 🅱️ introducing sensible defaults as prebuilds make more sense for the default branch which was probably the most common context for new workspaces.

❓ question: Maybe we could a) gather some usage data metrics or b) get an implementation estimation before making the final call?

Keeping in mind that there are overhead costs to creating and tracking new issues, let's avoid spawning followup issues to followup issues, when asking for clarification in the issue at hand would suffice.

Fair point! 🤝

I still think there's a lot of value in breaking tasks is smaller follow-up issues in case the implementation overhead increases the risk of scope creep for a PR, even if the initial PR solves the follow up issue, but I agree this could be tackled in #7367.

❓ question: Do you think we could try to tackle also #4353 within #7367?

…ted gitlab

AlexTugarev · 2022-01-04T12:00:40Z

@jldec and @gtsiolis, great questions and discussion so far, and I'm looking forward to clean up the differences between repo providers 🙏🏻 obviously, it makes sense to have a similar behavior from the product perspective.

for the scope of this PR, I'd like to move forward if you think it solves the two mentioned issues as described.

jldec · 2022-01-04T12:09:55Z

Yes, thanks @AlexTugarev

I was able to validate that prebuilds are triggered on a new project for Bitbucket. I don't believe Enable prebuilds for the default branch when adding a project for a GitLab or Bitbucket repository #7424 is valid.
I will try with GitLab self-hosted as well if you could register an instance.

AlexTugarev · 2022-01-04T12:13:25Z

I will try with GitLab self-hosted as well if you could register an instance.

We can try with the internal test installation, though it's not required because the implementation now checks for the provider type (and not for the host name,) so if it works with gitlab.com it should work with mygitlab.com as well.

AlexTugarev · 2022-01-04T12:14:39Z

components/server/src/projects/projects-service.ts

-            const repositoryService = this.hostContextProvider.get(parsedUrl?.host)?.services?.repositoryService;
+        const hostContext = parsedUrl?.host ? this.hostContextProvider.get(parsedUrl?.host) : undefined;
+        const type = hostContext && hostContext.authProvider.info.authProviderType;
+        if (type === "GitLab" || type === "Bitbucket") {


@jldec, this is the change which treats all GItLab installations equal, so testing with gitlab.com should be enough.

ok - i'll wait for staging to test :)

jldec · 2022-01-04T12:21:55Z

/lgtm

roboquat · 2022-01-04T12:21:59Z

LGTM label has been added.

Git tree hash: 19b26abbdb22da5b2a4c85c92f9643985a4463e2

roboquat · 2022-01-04T12:22:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gtsiolis, jldec

Associated issue: #7367

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~~ [gtsiolis,jldec]

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

jldec · 2022-01-04T15:42:02Z

/unhold

roboquat added do-not-merge/release-note-label-needed team: webapp Issue belongs to the WebApp team size/XS labels Jan 3, 2022

AlexTugarev requested a review from gtsiolis January 3, 2022 14:48

roboquat added release-note and removed do-not-merge/release-note-label-needed labels Jan 3, 2022

gtsiolis approved these changes Jan 3, 2022

View reviewed changes

roboquat added the do-not-merge/hold label Jan 3, 2022

roboquat assigned gtsiolis Jan 3, 2022

roboquat added the lgtm label Jan 3, 2022

roboquat added the approved label Jan 3, 2022

AlexTugarev mentioned this pull request Jan 4, 2022

Adding a project for a Bitbucket repository will not trigger any prebuilds or add any webhooks #7367

Closed

[projects] enable webhook registration for bitbucket.org and self-hos…

078674d

…ted gitlab

AlexTugarev force-pushed the at/bb-webhooks branch from 4e83ecf to 078674d Compare January 4, 2022 09:36

roboquat removed the lgtm label Jan 4, 2022

AlexTugarev changed the title ~~[projects] enable webhook registration for bitbucket.org~~ [projects] enable webhook registration for bitbucket.org and self-managed gitlab Jan 4, 2022

AlexTugarev requested a review from gtsiolis January 4, 2022 10:00

AlexTugarev commented Jan 4, 2022

View reviewed changes

roboquat assigned jldec Jan 4, 2022

roboquat added the lgtm label Jan 4, 2022

roboquat removed the do-not-merge/hold label Jan 4, 2022

roboquat merged commit 911d00e into main Jan 4, 2022

roboquat deleted the at/bb-webhooks branch January 4, 2022 15:43

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jan 6, 2022

jankeromnes mentioned this pull request Feb 8, 2022

Remove deprecated '#prebuild/' context URL prefix #4353

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[projects] enable webhook registration for bitbucket.org and self-managed gitlab #7422

[projects] enable webhook registration for bitbucket.org and self-managed gitlab #7422

AlexTugarev commented Jan 3, 2022 •

edited

Loading

gtsiolis commented Jan 3, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

gtsiolis Jan 3, 2022

AlexTugarev Jan 4, 2022

gtsiolis Jan 4, 2022

roboquat commented Jan 3, 2022

gtsiolis commented Jan 3, 2022 •

edited

Loading

jldec commented Jan 3, 2022

gtsiolis commented Jan 3, 2022 •

edited

Loading

AlexTugarev commented Jan 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jldec commented Jan 4, 2022

AlexTugarev commented Jan 4, 2022

AlexTugarev Jan 4, 2022

jldec Jan 4, 2022

jldec commented Jan 4, 2022

roboquat commented Jan 4, 2022

roboquat commented Jan 4, 2022

jldec commented Jan 4, 2022

[projects] enable webhook registration for bitbucket.org and self-managed gitlab #7422

[projects] enable webhook registration for bitbucket.org and self-managed gitlab #7422

Conversation

AlexTugarev commented Jan 3, 2022 • edited Loading

gtsiolis commented Jan 3, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment • edited Loading

Choose a reason for hiding this comment

gtsiolis Jan 3, 2022

Choose a reason for hiding this comment

AlexTugarev Jan 4, 2022

Choose a reason for hiding this comment

gtsiolis Jan 4, 2022

Choose a reason for hiding this comment

roboquat commented Jan 3, 2022

gtsiolis commented Jan 3, 2022 • edited Loading

jldec commented Jan 3, 2022

gtsiolis commented Jan 3, 2022 • edited Loading

AlexTugarev commented Jan 4, 2022 • edited by werft-gitpod-dev-com bot Loading

jldec commented Jan 4, 2022

AlexTugarev commented Jan 4, 2022

AlexTugarev Jan 4, 2022

Choose a reason for hiding this comment

jldec Jan 4, 2022

Choose a reason for hiding this comment

jldec commented Jan 4, 2022

roboquat commented Jan 4, 2022

roboquat commented Jan 4, 2022

jldec commented Jan 4, 2022

AlexTugarev commented Jan 3, 2022 •

edited

Loading

gtsiolis commented Jan 3, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

gtsiolis commented Jan 3, 2022 •

edited

Loading

gtsiolis commented Jan 3, 2022 •

edited

Loading

AlexTugarev commented Jan 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading