lorawan-stack icon indicating copy to clipboard operation
lorawan-stack copied to clipboard

feature: Pause application webhooks

Open vlasebian opened this issue 1 year ago • 5 comments

Summary

References #7223

Changes

  • Add a paused boolean field to the ApplicationWebhook object
  • Skip processing webhooks with the paused field set to True

Testing

Steps
  • Create an application
  • Create an end device
  • Create a webhook (webhook.site can be used to set up one) using:
curl -H "Authorization: $AUTH_TOKEN" 'http://localhost:1885/api/v3/as/webhooks/app1' -d '{"webhook":{"base_url":"https://webhook.site/1e8cc8cf-1b46-4da5-92ea-711b818ba6ec","paused": false,"field_mask":{"paths":[]},"format":"json","ids":{"webhook_id":"wh4"}},"field_mask":{"paths":["base_url","field_mask","format","ids.webhook_id","paused"]}}'
  • Simulate an uplink message and check if it's received by webhook.site
  • Update the webhook to pause it using:
curl -H "Authorization: $AUTH_TOKEN" 'http://localhost:1885/api/v3/as/webhooks/app1/wh4' -X PUT -d '{"webhook":{"base_url":"https://webhook.site/1e8cc8cf-1b46-4da5-92ea-711b818ba6ec","paused": true,"field_mask":{"paths":[]},"format":"json","ids":{"webhook_id":"wh4"}},"field_mask":{"paths":["base_url","field_mask","format","ids.webhook_id","paused"]}}'
  • Simulate another uplink message, no webhook should be received this time
Regressions

I checked if an existing webhook created in a previous version is affected by this change. This are the steps that I did:

  • Checked out on the v3.32 branch
  • Launched the stack and created an app, a device and a custom webhook
  • Sent a simulated uplink and checked if it was sent to the webhooks site
  • Shut down the stack
  • Checked out on the feature branch
  • Launched the stack
  • Sent a curl request to pause the webhook
  • Sent a simulated uplink, it wasn't sent anymore
  • Sent a curl request to unpause the webhook
  • Sent a simulated uplink and checked if it was sent to the webhooks site

Notes for Reviewers

Both uplink and downlink webhooks are paused if the flag is set.

Checklist

  • [x] Scope: The referenced issue is addressed, there are no unrelated changes.
  • [ ] Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • [ ] Documentation: Relevant documentation is added or updated.
  • [x] Testing: The steps/process to test this feature are clearly explained including testing for regressions.
  • [x] Infrastructure: If infrastructural changes (e.g., new RPC, configuration) are needed, a separate issue is created in the infrastructural repositories.
  • [x] Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • [x] Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

vlasebian avatar Aug 19 '24 08:08 vlasebian

Code looks good to me.

I guess we want Console support for this as well?

Do we need to add an end-to-end test?

Backlog is not a good milestone for a PR.

johanstokking avatar Aug 23 '24 12:08 johanstokking

For now I placed it in the Backlog milestone as the issue is also in Backlog.

vlasebian avatar Aug 27 '24 08:08 vlasebian

Regarding milestones, backlog is indeed sort of a bucket for issues waiting to be picked up. Once you decide to pick up something, always move the issue to the proper version milestone and set the in progress label.

KrishnaIyer avatar Aug 28 '24 13:08 KrishnaIyer

I think we should trigger a notification to the users when a Webhook is paused or unpaused. I don't think we currently have a mechanism for TTS components to trigger a notification in the IS since CreateNotificationRequest is currently only used internally by the IS. @nicholaspcr is this correct?

@vlasebian: On second thought, let's skip this. This would make this PR more complex than it is. I would however like to emit an event when the user pauses/unpauses a webhook.

KrishnaIyer avatar Aug 29 '24 06:08 KrishnaIyer

Does this call actually work? I think we require bearer tokens so Authorization: Bearer <token>

It is there, yes, in the curl -H "Authorization: $AUTH_TOKEN" part. To get the token I usually log in the console and check the network tab in the dev tools to get the Bearer token. Then I do export AUTH_TOKEN=Bearer ... so I have the token for all the requests in my terminal.

vlasebian avatar Aug 29 '24 10:08 vlasebian