Updates to Webhook events and filtering

[philipnbbc]

Details

This PR makes the following changes to the webhook specified in TAMS:

• The flows/segments_added event now includes the list of Flow Segment data structures (segments) rather than just a timerange.
• Flow and Flow Segment events can be filtered by Source ID (source_ids webhook option now also applies to Flows / Flow Segments).
• Flow and Flow Segment events can be filtered by Flow Collections that Flows are collected_by (flow_collected_by_ids webhook option).
• Flow, Flow Segment and Source events can be filtered by Source Collections that the (Flow's) Source are collected_by (source_collected_by_ids webhook option).

This PR does not provide a means to transform the events based on options in the webhook registration. For example, being able to disable inclusion of presigned get_urls in Flow Segments. However, if the motivation was to not burden the TAMS service with creating presigned URLs then it needs to be disabled for all webhooks rather than be a per-webhook option.

Jira Issue (if relevant)

Jira URL: https://jira.dev.bbc.co.uk/browse/CLOUDFIT-3547 and https://jira.dev.bbc.co.uk/browse/CLOUDFIT-5377

Related PRs

Where appropriate. Indicate order to be merged.

Submitter PR Checks

(tick as appropriate)

• PR completes task/fixes bug
• API version has been incremented if necessary
• ADR status has been updated, and ADR implementation has been recorded
• Documentation updated (README, etc.)
• PR added to Jira Issue (if relevant)
• Follow-up stories added to Jira

Reviewer PR Checks

(tick as appropriate)

• PR completes task/fixes bug
• Design makes sense, and fits with our current code base
• Code is easy to follow
• PR size is sensible
• Commit history is sensible and tidy

Info on PRs

The checks above are guidelines. They don't all have to be ticked, but they should all have been considered.

[samdbmg]

We had a call about this:

Option 3 seems preferable if achievable

• If we didn't have to worry about how to implement it, it is the option that provides the most flexibility and options for a client, and aligns with how GET /flows/<flowid>/segments works already regarding accept_get_urls
• There may also be a security aspect: it's possible clients could exist that need segment updates, but cannot be allowed access to the content (and therefore cannot receive presigned URLs in their webhooks)
• We discussed implementation approaches using AWS EventBridge
  • It's possible use the POST /service/webhooks to register a Target and Rule in EventBridge to deliver the webhook, and then use the state of the rules to store details of registered webhooks. Then events are delivered to EventBridge by the store implementation, which forwards them on according to those rules
  • Or you could send all events, and call something like an AWS Lambda to read the list of registered webhooks in the stores backing database, and make HTTP requests accordingly
  • Or you could step away from AWS EventBridge and make that DB query/HTTP requests process a function that is triggered in the background inside an API server
  • Clearly the first one may make filtering (to adjust the mix of get_urls sent to each webhook target) more difficult, and may require a switch to AWS EventBridge Pipes (with an Enrichment Lambda) or some other approach. However the other two approaches see little difference when the accept_get_urls filtering is added
• Could an implementation ignore accept_get_urls and supply them all anyway?
  • We could allow an implementation to do that, however it should be clear that's the case
  • We could add wording to the effect that "Implementations MAY choose not to implement accept_get_urls filtering of webhooks; in that case if it is requested when a webhook is registered, the implementation MUST respond with a 400 error` (or other suitable HTTP response code)

Following of Flow and Source collections

• Traversing a large hierarchy to identify whether an event should be sent to a subscriber may be an expensive operation
• As described here, it only operates on a single level (e.g. "give me events for all Flows directly collected by this Flow") however that isn't immediately obvious on a first read: we should make it more obvious in the wording

[philipnbbc]

I've made the following changes to the proposal following the call:

• Added accept_get_urls to the proposal
• Explained which Flow and Source properties are required to filter events
• Specified that API implementation return a 400 error if the requested webhook event filtering or transformation is not supported

[danjhd]

I noticed that the accept_get_urls field has been added to webhook-post.json but not to webhook.json? (the response to the GET). Was that deliberate?

[philipnbbc]

I noticed that the accept_get_urls field has been added to webhook-post.json but not to webhook.json? (the response to the GET). Was that deliberate?

Oops. no. Thanks for spotting that. Fix squashed into the commit.

[j616]

LGTM. Should this be sem-ver: api-break due to the change to the segments added webhook body?