feat(alerting): add email and click action to ntfy provider

[stendler]

Summary

Adds the email and click properties to the ntfy provider.

• email - will optionally send the notification also to this email
• click - will optionally add a link to the notification to be clicked on (e.g., a link to the gatus instance)

Documentation to publishing on ntfy via JSON: https://docs.ntfy.sh/publish/#publish-as-json

Edit: Now also adds the properties firebase and cache, which currently cannot be defined in JSON but as http headers (binwiederhier/ntfy#1119).

Checklist

• Tested and/or added tests to validate that the changes work as intended, if applicable.
  • manually tested configurations with and without these properties
• Updated documentation in README.md, if applicable.

[stendler]

There are also other properties supported by ntfy, which are not implemented yet:

• actions to define custom action buttons for the notificaton
• attach - URL to an attachment (sounds less useful for gatus)
• markdown (probably breaks with the current message content, especially containing patterns)
• icon URL (could be useful for someone)
• delay
• call (requires a authentication to test)

https://docs.ntfy.sh/publish/#publish-as-json

There are also cache and firebase supported using the http request instead of JSON. Not sure if they are also supported via JSON. Those are currently only supported via http request headers (see binwiederhier/ntfy#1119 & binwiederhier/ntfy#1123).

That and other not implemented properties could be worked around via the custom provider for now.

[TwiN]

Looks good, but could you create a few tests?

[TwiN]

There are also other properties supported by ntfy, which are not implemented yet:

* [`actions`](https://docs.ntfy.sh/publish/#action-buttons) to define custom action buttons for the notificaton

* `attach` - URL to an attachment (sounds less useful for gatus)

* `markdown` (probably breaks with the current message content, especially containing patterns)

* [`icon`](https://docs.ntfy.sh/publish/#icons) URL (could be useful for someone)

* `delay`

* [`call`](https://docs.ntfy.sh/publish/#phone-calls) (requires a authentication to test)

https://docs.ntfy.sh/publish/#publish-as-json

There are also cache and firebase supported using the http request instead of JSON. Not sure if they are also supported via JSON.

That and not implemented properties could be worked around via the custom provider for now.

@stendler WRT Not sure if they are also supported via JSON - if you're unsure, would it be possible to omit those features from this PR? I'd rather just include the features we are 100% sure works.

[stendler]

WRT Not sure if they are also supported via JSON - if you're unsure, would it be possible to omit those features from this PR? I'd rather just include the features we are 100% sure works.

Sorry, I have not updated that comment but only the PR summary: cache and firebase are not supported in the JSON payload, BUT as http request headers. That's what I've done in the implementation.

Looks good, but could you create a few tests?

What kind of tests do you want to see here? For TestAlertProvider_buildRequestBody a new scenario for the new JSON attributes, or add them to an existing one?

There does not seem to be a test for the request headers there yet. So either a test with a server receiving the request is needed or a refactor of the Send method into a buildRequest to be able to test for the headers.

Which of these options do you think are best?

[TwiN]

@stendler A new case for TestAlertProvider_buildRequestBody should suffice!

[stendler]

@TwiN I've now added tests for the json properties and also tests for verifying the request headers from the Send function using httptest.

[TwiN]

I'm almost wondering if it would make more sense to rename these to disable-firebase and disable-cache respectively so that we can have both of them just be bool instead of *bool.

What do you think?

[stendler]

Yeah, that sounds good. This would also make these config options a bit more self-explanatory.
Will do that!

[stendler]

@TwiN hmm, now that I've slept over this: there is a niche edge case here. Omitting to set the headers (which I do here if disabled) will result in the server default. Which is enabled for ntfy.sh, but might be different for other instances (if they support this at all).

So either

• always set the headers explicitly, defaulting to enabled if this config is missing
• only set explicitly if it should be disabled and omit otherwise, falling back to the server default (current implemented behavior)
• use nil to use the server default and set explicitly if the user specifies so in the config

The first two can avoid the pointer, while the latter option needs it to determine if this option is set in the config or not.
I would tend to the last option for more flexibility, although it might be unlikely to matter in practice.
If you definitely want to avoid the pointer, that's fine by me as well.

[TwiN]

I see your point.

Am I correct in my understanding that for firebase to work in the first place, multiple configurations on ntfy's side must be set, and even if firebase is enabled through Gatus, without these configurations on ntfy's side, firebase will not work anyways?

If so, I think we can assume the intent of the user without needing a pointer:

• If disable-firebase is set to false (default), then the Firebase header is present with the value yes and will work BUT ONLY if the user configured firebase to work on ntfy, which is the behavior of the current implementation
• If disable-firebase is set to true, then the Firebase header is set to no, which the user would only explicitly set disable-firebase to true if their ntfy configuration had firebase configured and they didn't want this Gatus alert to use Firebase.

Does that make sense?

P.S. This assumes that a request with Firebase: yes won't return a 4xx error if the ntfy configuration doesn't have Firebase on it but will instead fail silently.

[stendler]

Yes, your understanding is correct. For self-hosted, the admin needs to explicitly configure it (it apparently even requires a modified android app).
I've also checked that:

• If not configured, but the header is set, it is silently ignored
• If the header is not set, regardless of server configuration, it is set to a hard-coded default of true - so my assumption, that the default might differ on other instances, was false

With the naming changed to disabled-firebase, I would suggest omitting the header if the config is omitted or set to false. Especially, since enabled is the hard-coded default on the server anyway.

I've now updated this PR this way.

[stendler]

ping @TwiN in case you missed my last message