Brigade: [Design] CloudEvents support for Generic Gateway

Created on 6 Feb 2019  路  10Comments  路  Source: brigadecore/brigade

This is the design proposal for CloudEvents support for Generic Gateway, as discussed in #764

Currently, Generic Gateway listens to "/webhook" for a simple JSON payload and will raise a "webhook" event that should be handled by brigade.js.

Generic Gateway could support latest version of CloudEvents v0.2. As per proposed implementation, Generic Gateway could listen to "/cloudevent" for such an event and handle it via a "cloudevent" event handler on brigade.js.

Some points:

  1. Relevant Brigade data (e.g. ref and commit) could be stored on Event's Data field. Brigade.js consumer should be able to see the entire payload, of course.
    2 CloudEvents could support the existing security mechanism we already have for Generic Gateway (with a custom secret per Project)
  2. It should be made clear in the documentation that Generic Gateway supports a Generic webhook endpoint as well as a CloudEvents endpoint. In this context, maybe rename the Generic webhook endpoint to simple webhook?
  3. Brigade should have a dependency to github.com/cloudevents/sdk-go/v02

Thoughts?

design

Most helpful comment

I think it makes sense to add another endpoint to the existing gateway.

All 10 comments

I think the design described here would be a great first iteration!

My only thoughts are at a higher-level around the future of gateways and Brigade. I'm curious if it would be desirable that any additional gateways besides "legacy" (container registry, generic) should exist in separate repos and be pluggable by way of sub-charts to the main Brigade chart?

We're exploring this approach for the Github gateway as we transition to using a pluggable but separate brigade-github-app for this gateway functionality (and thus, pulling out the classic GH gateway code; see https://github.com/Azure/brigade/pull/797)

Each additional gateway being built as a standalone project might be preferable _if_ Brigade is in a position to support this (or could be with a small amount of work). I suppose one drawback of standalone-but-composable gateways might be separate service IPs for each and the overhead of covering each w/ ingress/TLS?

However, as @dgkanatsios has the most recent experience with gateway work, I'd definitely defer to his experience and opinions around this idea.

@vdice - even with the current generic gateway, we have a separate IP for the service - the way we could help here would be to document how to properly set up an ingress with TLS, then expose a single public IP for your ingress, and not one for all gateways.

That being said, I fully agree on having multiple repos for gateways and composing them at the chart level.

thanks both for comments/review! A few things, mainly on context/naming:

  1. @vdice you mention generic gateway. Just to be on the same page, I suppose you mean the single generic gateway with the two webhooks (the simple one - which is already implemented - and the cloudevents one - which this issue tries to design)
  2. also @vdice you mention that generic gateway is considered legacy. I suppose again you mean the single generic gateway with the two endpoints (simple and cloudevents), correct? So, you think that the generic gateway should stay in the brigade repo?
  3. If the answer to (1) and (2) is false, then are we considering a separate gateway for cloud events? So the idea is to have two gateways, the already existing one and the one designed by this issue?
  4. @radu-matei yup, a simple tutorial with nginx/cert-manager for TLS and a couple of gateways behind that would be ideal!

I personally do agree that it would be good from an organizational perspective to have all gateways on their separate repos; I don't really see any cons on that. We could also create a fake gateway (on its separate repo, of course) that could serve as a starting point for people that want to design their own gateway (with their custom events etc.).

Looking forward to your responses on 1,2,3 to see how we will proceed.

  1. I indeed had in mind the current generic gateway with the one, current simple webhook

  2. I'm using the legacy term here to denote gateways that were added directly to the Brigade project itself before we started following the pattern of creating them in separate repos/projects and including them into a Brigade setup at time of install.

  3. Indeed, I was thinking that any new gateway would be built as a separate project and included at install time (read: added as an optional sub-chart to the Brigade chart). Naturally, since this is still just being discussed, we'd want to put this in writing in Brigade's docs before expecting this approach to be followed by contributors.

That being said, I think I didn't quite understand the vision of the generic gateway until the helpful clarifications above. If this gateway was always intended to grow in utility via additional endpoints (cloudevents being the next), then I can definitely see just adding this logic to the gateway itself, in this repo.

If for some reason a new endpoint made more sense to exist in the context of a separate/distinct new gateway, this is when I'd propose it live via a separate project. (In fact, down the road, I wonder if we'll want to break out the existing Container Registry gateway into a separate repo/chart.)

got it, thanks @vdice! so, the question is whether to add 'cloudevents' endpoint to the existing generic gateway or create a completely new gateway. Some thoughts:

  • if we the 'cloudevents' endpoint to the existing generic gateway, we do have the advantage that we will not need to generate another secret that protects the gateway access (the one set via GET here)
  • in my mind, both simple gateway and the cloudevents one are pretty similar, in the sense that each one just accepts a JSON payload from an external caller. The only difference is that the cloudevents one is better defined and follows the CloudEvents standard
    for the above two reasons, I would vote to extend the generic gateway with a 'cloudevents' endpoint, so as a result, there will be a single gateway that will have two endpoints, the 'simple' and the 'cloudevents' one.
    I'd love to hear what everyone else thinks, though!

PS. Once we decide about it, we can discuss where to write the code (here or in a different repo)

I think it makes sense to add another endpoint to the existing gateway.

I think it makes sense to add another endpoint to the existing gateway.

Agreed! Thanks for helping me understand.

thanks both, I'll work towards a PR for it on this repo. Maybe after v1.0 we can take the entire generic gateway out.

Sure, we can look at having a separate repo in the new org.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tamizharasanr picture tamizharasanr  路  4Comments

mattfarina picture mattfarina  路  4Comments

radu-matei picture radu-matei  路  6Comments

sathyatvrcbe picture sathyatvrcbe  路  6Comments

carolynvs picture carolynvs  路  5Comments