Outgoing hooks for Batch Changes

[malomarrec]

Problem

One of the main problem that Batch Changes solves for users is making sure large campaigns with many changesets get merged efficiently, with minimal work from the batch change creator. A key step to get there is to make sure repo owners are aware of the changes, in their context. Customers have a very heterogenous set of tooling and practices to make that happen: Jira, a custom tool, codeowners, custom systems.

Solution

We think that a good approach to solving this problem is to start by making it very easy for customers to integrate with their tooling. Then, depending on commonly build integrations, we can build some integrations ourselves either as customizations maintained by solutions engs, as extensions or into the core product.

That's why we think the reasonable first step is to build outgoing webhooks.

See a proposal here, discussions in https://github.com/sourcegraph/sourcegraph/issues/26790.

Solution validation

This approach is validated at a high level because:

• CEs customers have already built rough scripts to integrated with their system. Webhooks would considerably improve this. In particular, some customers use solutions that are unable to keep their ticketing system in sync with the status of changesets: it opens tickets, but never closes them which becomes pathological at 1k changeset scale.
• While every customer would prefer a native integration for their own system, they assess that making it easier to build their own integration is a great first step.
• This rough solution is in use at a customer and could be retired in favor of a better solution using webhooks.

Delivery plan

• [ ] Share and validate plan with other eng teams (RFC), so that we have a single outgoing hooks platform
• [ ] Validate solution with 3 customers
• [ ] Get feedback from implementation engineers (on using this to build custom workflows for key customers)

[chrispine]

unsure of priority on this one; partially depends on the estimate, and on Malo's thoughts

[malomarrec]

Given the fact that we're now focusing on making batch changes create immediate value to most devs, on top of server-side and all the rest, I (regretfully) suggest we push this back to Q4.

[LawnGnome]

Alas, I only have about an hour left before I am dead. 💀

(Well, only in Sourcegraph terms. I'm fine. Really!)

Overall project status

This is in generally good shape. I expect there will be seven PRs, of which six are open and ready for review. I've exercised the backend of this pretty thoroughly, and the site admin UI to a reasonable extent, and it feels pretty solid to me.

I estimate there are about 1-2 days of work remaining: a mixture of documentation writing and frontend polish.

Testing outbound webhooks

46105 should be fully functional.

45833 was the original integration branch I used while developing this — it should be functionally equivalent to #46105, but if any archaeology needs to happen, that's the branch to dig into.

PRs

Here are the PRs I've opened:

• 46007

• 46010

• 46011

• 46012

• 46013

• 46015

They are stacked on top of each other, and need to be reviewed and merged in that order. (GitHub should be smart enough to retarget each PR as the earlier ones are merged, but let's not pretend this is super reliable.) I expect the first five PRs are substantially complete and ready to merge. The sixth PR implements the site admin UI for outbound webhooks — it's functional, but I think someone more in tune with our TypeScript/React norms would be able to tidy it up somewhat, and it could use a minimal design review.

What's not in a PR

The thing I've run out of time to do is write useful documentation. (I'm sorry. I really did have good intentions here.) That would be the mysterious PR 7 I mentioned earlier.

Things I expect would need to be in user facing documentation:

• Basic "how to configure webhooks" documentation once #46105 has had a design review.
• A page describing the event types available in #46012, and what the payloads look like. (There are only two payload types right now!)
• The webhook log retention configuration added in #27179 has been reused for outbound webhooks, so whatever references we have to that in the docs should be updated to note that it also applies to outbound webhook logs.

Bonus points for internal documentation noting what the high level API in #46011 looks like, and how #46012 uses that to implement Batch Changes webhooks. I suspect other product areas will eventually want their own webhooks, and it should be easy to write a one page developer-focused document using #46012 as an example, much like the developing a worker page.

What the hell is a scope, anyway?

The good news is that I designed the implementation to be cleanly reusable by basically any part of the Sourcegraph product, enterprise or OSS. Register one or more event types, call OutboundWebhookService.Enqueue at the appropriate moments, and things will just work.

One thing that we don't need but most other parts of the product will need is the ability to scope webhooks to specific entities. What's implemented right now is basically the equivalent of a site-wide GitHub webhooks — useful for an MVP, but a very blunt instrument.

As a result, I've implemented low level support for event types and events to have an optional scope. This is an opaque string, to be defined on an event type specific basis, that can be used to only send webhooks to specific subsets of the outbound webhooks registered for an event type.

I've intentionally not exposed this in the site admin UI for now, although the GraphQL queries and mutations do have this ready to go. What's missing are two things:

1. Site admin components to expose outbound webhook pages for specific entities (such as for a single repo).
2. A way of registering an event type that doesn't show up on the global site admin page. (This would probably be as simple as extending outbound.EventType with an extra optional string field that can be used for scoping in its own way.)

This isn't part of this MVP, and I wouldn't recommend doing further work on this right now. However, you can point someone who wants to add, say, repo-specific outbound webhooks at the last few paragraphs and I suspect they could quickly implement the missing pieces.

[evict]

@courier-new would it be possible to add some sort of outgoing request filter here? Something like Gitea's hostmatcher?

You can make a new context that does filtering like so: https://github.com/sourcegraph/sourcegraph/pull/42108/files#diff-39024228d118dcefb9d91855eadc6faf8aa279ec71ef0f7c078981c37d3ecea8.

[courier-new]

Hey @evict, yeah absolutely! I was actually just about to tag you to review https://github.com/sourcegraph/sourcegraph/pull/46011/files?w=1#diff-1a8893f9a3da19a47f22ff92287f6da213facbc7780d344d836197402d13dc15R56-R114, which is essentially my attempt at hardcoded, manual request address filtering. I could probably clean that up some with hostmatcher. 😅 We're also intentionally filtering other addresses out, too. Let me clean that up a little bit but then I'd love your review there. 🙂 Do you think it's important that admins are able to configure the allow/denylist at this time, or is it okay if we (Sourcegraph) are the only ones setting the rules for now?

[evict]

This looks good! I do think that we should be the ones setting the rules and ideally it's something not directly configurable as a site-admin. But this does reduce user-friendliness.