Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support for Telegram #33

Closed
djmattyg007 opened this issue May 15, 2022 · 45 comments · Fixed by #63
Closed

feat: Support for Telegram #33

djmattyg007 opened this issue May 15, 2022 · 45 comments · Fixed by #63
Labels
enhancement New feature or request

Comments

@djmattyg007
Copy link

Is your feature request related to a problem? Please describe.
I don't use Slack or Gotify.

Describe the solution you'd like
I'd like to be able to specify an arbitrary set of Telegram bot API credentials, as well as an arbitrary chat ID. Messages should arrive at that chat.

Describe alternatives you've considered
I tried looking for a Tekegram plugin for Gotify, but was unsuccessful.

@djmattyg007 djmattyg007 added the enhancement New feature or request label May 15, 2022
@djmattyg007
Copy link
Author

Perhaps consider using a library like https://github.com/nikoksr/notify ?

@samcro1967
Copy link

Might want to look at apprise. Pretty large library of supported options.

@JosephKav
Copy link
Collaborator

apprise is a Python module, so can't use that sadly.
I don't really see the use of that notify library. You still have to implement the services yourself, you just then give them to that notify and send with that. So you'd still need to add support for every service yourself.
I'll work on adding Telegram support later on 👍

@samcro1967
Copy link

Shoutrrr is another alternative written in go. It does not have as many services, but might also be worth a look.

@djmattyg007
Copy link
Author

Using a library like notify lets you avoid all the implementation details and use a simple interface in your own code. The alternative is learning the minutiae of every single service's request formats and the like.

@JosephKav
Copy link
Collaborator

JosephKav commented May 15, 2022

Shoutrrr is another alternative written in go. It does not have as many services, but might also be worth a look.

This looks more preferable to notify (imo). Thinking I'll replace slack: and gotify: with notify: (that uses Shoutrrr not Notify). Then make that read yaml like this

service:
  release-argus/argus:
    ....
    notify:
      slack_example:
        type: slack
        token: xyz
        channel: something
        params:
          botname: argus
          color: #ff8000
          icon: github
          title: New release

gonna do this instead so that I don't need to define every param as a type when they get added/removed from Shoutrrr
Nvm, I can just do the above as a string map

        params:
          - key: botname
            value: argus
          - key: color
            value: #ff8000
          - key: icon
            value: github
          - key: title
            value: New release

@djmattyg007
Copy link
Author

It would be nice if we could specify multiple different notification targets, but that’s just a nice to have.

@JosephKav
Copy link
Collaborator

It would be nice if we could specify multiple different notification targets, but that’s just a nice to have.

What do you mean by 'different notification targets'? You can (and will still) be able to setup multiple of each type of notification to go to different places.
I have Shoutrrr mostly working, am just working on automated conversions to the new format for existing slacks (to slack/mattermost) and gotify. Am also trying to figure out how the Shoutrrr code works so that I can PR icon support for Mattermost

@samcro1967
Copy link

@djmattyg007 Look into mailrise. It is an SMTP gateway that uses apprise under the covers. Use Shoutrr to send email to mailrise and use mailrise to then forward. It allows you to setup different email addresses. Each can have one or more destinations. I use this for all of my apps. Each app has its own channel in Discord and Gotifty. Some I send emails. You then have one config for all of your apps. If you change email addresses, you have one place to go to change all of your apps. Or if you want to change from Slack to Discord, one place to go. No longer have to remember every place you have to go make changes. Also opens up a lot more options of places to send.

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

After too many commits, I'd say that this is about ready in #63. Have implemented conversions of slack/gotify to not make this a breaking change, but I bet there will still be a config that breaks! :/. Gonna try some more testing of random configs tomorrow and ensure they all work.

Currently, switching to Shoutrrr is gonna lose us Gotify extras (unless me/someone else adds them to Shoutrrr), and until the next Shoutrrr release, we'll lose support for sending icons with our Mattermost webhooks
(have only tested slack/gotify/telegram. I believe the others should work, but I haven't tested them. Lucky I tested Telegram as the docs say that the chats var could be passed in 2 ways, but the way I wasn't doing is the only way that works 😢, have made an issue to track containrrr/shoutrrr#249)

@JosephKav
Copy link
Collaborator

Right I've done a bunch more testing and finally merged the PR. Tried to get conversions to handle places where you may have had a gotify and a slack named the same, but in the end I gave up as I thought a few edge cases would fail that way and I just want to get this out. It'll detect where slack is for slack by 'hooks.slack.com' being in the url and assume it's for mattermost otherwise. Then it will suffix all existing slack/gotify names with the type it detects, so they'll either end in '_gotify', '_mattermost' or '_slack', but I believe they should all continue working.

I'd appreciate some of you testing this and reporting back to me (with the log) if ur config fails with this new image.
The Docker :master image should be out in the next 10 mins or so

@samcro1967
Copy link

@JosephKav So Gotify notification would now look something like this using the shourtrr format for url's?

notify:
  GOTIFY_NOTIFICATION:
    url: gotify://ip_address:port
    token: <gotify_token>
    title: Argus Notification
    message: '{{ service_id }} - {{ version }} released.'
    extras:
      client_display: text/plain
      client_notification: '{{ web_url }}'
    priority: 5
    delay: 0s
    max_tries: 3

And each service would look like:

    notify:
      GOTIFY_NOTIFICATION: {}

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

@JosephKav So Gotify notification would now look something like this using the shourtrr format for url's?

@samcro1967

notify:
  GOTIFY_NOTIFICATION:
    type: gotify
    options:
      message: '{{ service_id }} - {{ version }} released.'
      delay: 0s
      max_tries: 3
    url_fields:
      host: ip_address
      port: 443
      token: <gotify_token>
    params:
      title: Argus Notification
      priority: 5

Forgot to link the updated docs, sorry! https://deploy-preview-13--release-argus.netlify.app/docs/config/notify/#gotify
(I'll merge them to release-argus.io once I do a release)
We've lost the extras ability with Gotify under Shoutrrr sadly

@samcro1967
Copy link

I updated config.yml and built and deployed a new local image. No errors in the logs. Will let you know what happens once an app is updated.

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

I updated config.yml and built and deployed a new local image. No errors in the logs. Will let you know what happens once an app is updated.

Ideally, you should have been able to use the existing config. Did you try that? You can test the notifiers with -test.notify NAME
It should check that the URL it'll give to Shoutrrr is valid on startup and error if it's not

@samcro1967
Copy link

I did not, but easy enough for me to roll back the config.yml. Argus starts fine with no errors. I see if added _gotify to my notification name. When I test it, I do get an error. It is trying to send an https request to an http server.

@JosephKav
Copy link
Collaborator

What url were you using?

@JosephKav
Copy link
Collaborator

I'm guessing (and hoping) one without http or https as I've made it default the port to 443 if it can't find those or a host:

@samcro1967
Copy link

just osu.gotify:8132.

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

Ah, so you'll need params.disabletls under this notify
(I'll make master do this automatically if it can't detect https)

@samcro1967
Copy link

samcro1967 commented May 25, 2022

I added DisableTLS: yes to the params and changed name to ip address to rule out any DNS weirdness. Still looks like it is trying https and not picking up the parameter. Also looks like it is saying can't find host. Double checked that I can ping by name and IP address from within the container.

docker exec -it argus bash -c "cd /etc/argus && argus -test.notify GOTIFY_NOTIFICATION"
INFO: Testing (GOTIFY_NOTIFICATION),
ERROR: GOTIFY_NOTIFICATION, failed to send notification to Gotify: Post "https://192.168.1.104:8132:443/message?token=<redacted>": dial tcp: lookup 192.168.1.104:8132: no such host

@JosephKav
Copy link
Collaborator

@samcro1967 What are you using as host in that one? Odd that it's got two ports

@samcro1967
Copy link

192.168.1.104:8132 Looks like you are automatically wrapping https: and 443 around whatever is entered into the Host field. Maybe the DisableTLS: yes is not working. I would think that should at least change https to http. You may want to add another url_field for port. You will have no way of knowing what port people are running services on. Would be cleaner I think to let folks know they have to specify both. Then you will know the port for sure and are not assuming it is on either 80 or 443
???

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

192.168.1.104:8132 Looks like you are automatically wrapping https: and 443 around whatever is entered into the Host field. Maybe the DisableTLS: yes is not working. I would think that should at least change https to http. You may want to add another url_field for port. You will have no way of knowing what port people are running services on. Would be cleaner I think to let folks know they have to specify both. Then you will know the port for sure and are not assuming it is on either 80 or 443 ???

There is one for port, it just defaults to 443. Looks like its not getting the params correctly, may be only in the testing call. Trying to fix currently

host: 192.168.1.104
port: 8132

The https bit isn't my code, Argus calls Shoutrrr with gotify://host:port/path/token, It's just not filling the params atm
fixed that and have a http gotify working

❯ make go-build && ./argus -config.file config.yml -test.notify gotify -log.level debug
>> compressing assets
>> building binaries
>> restoring assets
INFO: Testing (gotify),
DEBUG: gotify, Sending "TEST - NAME_OF_SERVICE - MAJOR.MINOR.PATCH released" to "gotify://gotify.example.io:444/<redacted>" with params=map["disabletls":"yes"]

@samcro1967
Copy link

Totally missed the port in the docs. I updated and it is now trying to get to "https://osu.gotify:8132.

@JosephKav
Copy link
Collaborator

Totally missed the port in the docs. I updated and it is now trying to get to "https://osu.gotify:8132.

Yeah, I've pushed a fix for that. The master tag should be updated in <10 mins. The params from defaults/notify weren't being passed down to the service notify

@JosephKav
Copy link
Collaborator

FYI, you can track the build of :master (happens every commit to the master branch) here

@samcro1967
Copy link

Test notification worked with the latest version and the new syntax. Do you want me to roll the config back to the old syntax and test that as well?

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

Test notification worked with the latest version and the new syntax. Do you want me to roll the config back to the old syntax and test that as well?

That'd be nice, but only if you wouldn't mind and it's not too much hassle 😄

@samcro1967
Copy link

The old config gave me the https to an http message.

docker exec -it argus bash -c "cd /etc/argus && argus -test.notify GOTIFY_NOTIFICATION_gotify"
INFO: Testing (GOTIFY_NOTIFICATION_gotify),
ERROR: GOTIFY_NOTIFICATION_gotify, failed to send notification to Gotify: Post "https://osu.gotify:8132/message?token=AdMn4Ycrgo8SUKu": http: server gave HTTP response to HTTPS client
ERROR: GOTIFY_NOTIFICATION_gotify, failed to send notification to Gotify: Post "https://osu.gotify:8132/message?token=AdMn4Ycrgo8SUKu": http: server gave HTTP response to HTTPS client
ERROR: GOTIFY_NOTIFICATION_gotify, failed to send notification to Gotify: Post "https://osu.gotify:8132/message?token=AdMn4Ycrgo8SUKu": http: server gave HTTP response to HTTPS client
ERROR: GOTIFY_NOTIFICATION_gotify, failed 3 times to send a gotify message to gotify://osu.gotify:8132/AdMn4Ycrgo8SUKu
ERROR: Testing (GOTIFY_NOTIFICATION_gotify), Message failed to send with "GOTIFY_NOTIFICATION_gotify" config
failed to send notification to Gotify: Post "https://osu.gotify:8132/message?token=AdMn4Ycrgo8SUKu": http: server gave HTTP response to HTTPS client x 3

Here is the old config:

gotify:
  GOTIFY_NOTIFICATION:
    url: http://osu.gotify:8132
    token: <redacted>
    title: Argus Notification
    message: '{{ service_id }} - {{ version }} released.'
    extras:
      client_display: text/plain
      client_notification: '{{ web_url }}'
    priority: 5
    delay: 0s
    max_tries: 3

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

@samcro1967

		url = strings.TrimPrefix(url, "http://")
		if strings.HasPrefix(url, "http://") {
			converted.SetParam("disabletls", "yes")
		}

🤦
I strip the http:// before I check for it and disabletls!
have amended the commit (also noticed an issue with the logger if you tried to test a notifier but didn't have a notify on any service)
Tested locally and it did set it to http. (Disabled http on my Gotify server as I thought it was all fixed)
ERROR: GOTIFY_NOTIFICATION_gotify, failed to send notification to Gotify: Post "http://localhost:8132/message?token=TOKEN": dial tcp 127.0.0.1:8132: connect: connection refused

@samcro1967
Copy link

Something else I should try?

@JosephKav
Copy link
Collaborator

JosephKav commented May 25, 2022

Something else I should try?

Not that I can think of? I'm saying it's all working as far as I can see w.r.t Gotify and HTTP. My command log was just to show that it did pick it up as http and disabletls, so tried http://. It's meant to fail as I pointed it at a non-existant server as I disabled http on my one after the testing earlier worked
Thanks for testing this!

@samcro1967
Copy link

So with the old config I specified http://osu.gotify:8132 as the URL. The notification was sent to https://osu.gotify:8132. It doesn't look like the strip is working to me with the old config, but maybe I am misunderstanding what you are saying.

@JosephKav
Copy link
Collaborator

So with the old config I specified http://osu.gotify:8132 as the URL. The notification was sent to https://osu.gotify:8132. It doesn't look like the strip is working to me with the old config, but maybe I am misunderstanding what you are saying.

Did you make sure to repull :master? I just tried on docker with your config and it worked. Changed my Gotify back to HTTP to confirm

❯ sudo docker exec -it c9265400f10d bash -c "cd /etc/argus && argus -test.notify GOTIFY_NOTIFICATION_gotify"
INFO: Testing (GOTIFY_NOTIFICATION_gotify), 
DEBUG: GOTIFY_NOTIFICATION_gotify, Sending "TEST - NAME_OF_SERVICE - MAJOR.MINOR.PATCH released." to "gotify://gotify.example.io:444/<SECRET>" with params=map["disabletls":"yes" "priority":"5" "title":"Argus Notification"]
INFO: Testing (GOTIFY_NOTIFICATION_gotify), Message sent successfully with "GOTIFY_NOTIFICATION_gotify" config

@samcro1967
Copy link

Just rebuilt the container and retestd. It worked. I wonder if I pulled it too soon after you made the last commit. Not sure if Github uses a CDN and maybe I had not replicated yet from where I pulled it. I will test discord tomorrow. :-)

@samcro1967
Copy link

Tested Discord and works as expected. Also tried email, but am getting an error which I am guessing it in my config, but I am not seeing it. I see MAILRISE_NOTIFICATION when I do a -config.check.

config.xml global block

notify:
  MAILRISE_NOTIFICATION:
    type: smtp
    url_fields:
      Host: osu.mailrise
      Port: 8025
    params:
      FromName: Argus
      FromAddress: argus@mydomain.net
      ToAddress: argus@mydomain.net
      Auth: None
      Subject: '{{ service_id }} Release'
      UseHTML: yes
      UseStartTLS: no

logs

docker exec -it argus bash -c 'cd /etc/argus && argus -test.notify MAILRISE_NOTIFICATION'
INFO: Testing (MAILRISE_NOTIFICATION),
ERROR: Testing (MAILRISE_NOTIFICATION), notify:
  MAILRISE_NOTIFICATION:
    ^ unknown service ""

@JosephKav
Copy link
Collaborator

Tested Discord and works as expected. Also tried email, but am getting an error which I am guessing it in my config, but I am not seeing it. I see MAILRISE_NOTIFICATION when I do a -config.check.

Ah, that's an inconsistency between the code and my docs. The code is expecting it as "email". I'll do a commit now to fix that (code will read and use smtp)

@samcro1967
Copy link

I built a new image and am now seeing this when I perform a test:

docker exec -it argus bash -c 'cd /etc/argus && argus -test.notify MAILRISE_NOTIFICATION'
INFO: Testing (MAILRISE_NOTIFICATION),
ERROR: Testing (MAILRISE_NOTIFICATION), notify:
  MAILRISE_NOTIFICATION:
    ^ toaddress is not a valid config key [auth encryption from fromaddress fromname starttls subject title to toaddresses usehtml]

@JosephKav
Copy link
Collaborator

JosephKav commented May 26, 2022

I built a new image and am now seeing this when I perform a test:

    ^ toaddress is not a valid config key [auth encryption from fromaddress fromname starttls subject title to toaddresses usehtml]

Ahh, Shoutrrr wants me to give ToAddresses not ToAddress! Oops again! Have amended the latest commit and fixed this. Thanks for testing!

@samcro1967
Copy link

Got beyond that error, but now getting this one:

ERROR: Testing (MAILRISE_NOTIFICATION), Message failed to send with "MAILRISE_NOTIFICATION" config
error applying params to send config: usestarttls is not a valid config key [auth encryption from fromaddress fromname starttls subject title to toaddresses usehtml] x 3

@JosephKav
Copy link
Collaborator

error applying params to send config: usestarttls is not a valid config key [auth encryption from fromaddress fromname starttls subject title to toaddresses usehtml] x 3

as you may have seen from the Shoutrrr PR i mentioned you in, this is an inconsistency with the Shoutrrr docs that I've proposed a fix for. I'll keep Argus to using usestarttls and just convert it to starttls internally for now. (will still save it as usestarttls)

@samcro1967
Copy link

I built a new image this morning and tested again. I am seeing the toaddresses issue again after the usestarttls commit.

 docker exec -it argus bash -c 'cd /etc/argus && argus -test.notify MAILRISE_NOTIFICATION'
INFO: Testing (MAILRISE_NOTIFICATION),
ERROR: Testing (MAILRISE_NOTIFICATION), notify:
  MAILRISE_NOTIFICATION:
    url_fields:
      toaddresses: <required> e.g. 'name@gmail.com'

@JosephKav
Copy link
Collaborator

JosephKav commented May 27, 2022

You need to make it toaddresses not toaddress in your params. I'll fix the PR for the new docs with this

I believe I originally put it as toaddress because I was typing with the Shoutrrr docs on the side and saw 'fromaddress' and beneath it 'toaddress...'. they're the same length so my brain just autocompleted it, missing off the 'es'. I'd say 'fromaddresses' makes more sense as you can give a comma separated list of addresses and so is more friendly from a user perspective?

@samcro1967
Copy link

Yup, it is working now. I can confirm Discord, Gotify, and email are all working with the implementation of Shoutrr. Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants