add support for generic webhooks #183
Conversation
Codecov Report
@@ Coverage Diff @@
## master #183 +/- ##
==========================================
- Coverage 55.48% 55.27% -0.21%
==========================================
Files 132 134 +2
Lines 4351 4400 +49
Branches 107 112 +5
==========================================
+ Hits 2414 2432 +18
- Misses 1937 1968 +31
Continue to review full report at Codecov.
|
| d: Datacenter.Deployment | ||
| ) extends NotificationEvent | ||
|
|
||
| object NotificationEvent { |
There was a problem hiding this comment.
So it looks like for webhooks you're sending big deploy and destroy blobs to the endpoint - I assume the intention is for (presumably custom) endpoints to then take that blob and do something useful with it? Put differently, this isn't specific to any specific kind of webhook for some service right?
There was a problem hiding this comment.
Precisely, the goal is to provide real-time context for events occurring within Nelson to any desired consumer.
| } | ||
|
|
||
| def send(s: WebHookSubscription, ev: NotificationEvent): IO[Unit] = | ||
| client.expect[String](Request[IO]( |
There was a problem hiding this comment.
Double checking, does client.expect[String] accept any response, or specifically only strings (e.g. if the webhook sent back a JSON blob will this fail? I seem to vaguely remember some edge cases here but maybe I'm crazy. Is there a way to make this fire-and-forget or something, if that is desirable?
There was a problem hiding this comment.
This is a great point, this should be fire and forget. We really shouldn't care about the response body, only the response code and that should only drive different logging behaviors imo.
| val env = plan.environment | ||
| val unit = ev.unit | ||
|
|
||
| ("namespace" := ns.name.asString) ->: |
There was a problem hiding this comment.
More a note for the future than a review comment, since we're moving to Protobuf (#159) we may want these as protocols as well, esp. since this is quite a bit of information. But fine for now :)
|
Realized this requires users to configure secrets into the manifest, which is unacceptable. I'll research having named hooks, which are set up in the nelson config, or allowing a configuration routine that fetches credentials securely from Vault. |
Adds support for webhook notifications with a minimal authorization provider implementation.