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

Slack: add properties for overriding channel and emoji #173

Closed
wants to merge 2 commits into from

Conversation

evenh
Copy link

@evenh evenh commented Jun 9, 2021

After looking into the new kured 1.7.0 release, I came across a couple of properties that I miss after the transition to shoutrrr. This PR introduces two new properties in the Slack service:

  • channel: allows for overriding which channel to be notified (default is using the channel specifying in the Slack webhook configuration)
  • emoji: use a custom icon for the messages posted by shoutrrr

Related to #88, kubereboot/kured#368

@piksel
Copy link
Member

piksel commented Jun 9, 2021

From the Slack incoming webhook documentation:

You cannot override the default channel (chosen by the user who installed your app), username, or icon when you're using Incoming Webhooks to post messages. Instead, these values will always inherit from the associated Slack app configuration.

@evenh
Copy link
Author

evenh commented Jun 9, 2021

That sounds like a retrofitted limitation on Slack's part. The existing implementation already support custom usernames. My patch works in practice and this approach has worked for kured for two years 🤔

@codecov
Copy link

codecov bot commented Jun 9, 2021

Codecov Report

Merging #173 (677cf61) into main (d8371b8) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #173      +/-   ##
==========================================
+ Coverage   63.08%   63.11%   +0.02%     
==========================================
  Files          79       79              
  Lines        2487     2489       +2     
==========================================
+ Hits         1569     1571       +2     
  Misses        780      780              
  Partials      138      138              
Impacted Files Coverage Δ
pkg/services/slack/slack_config.go 92.59% <ø> (ø)
pkg/services/slack/slack_json.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8371b8...677cf61. Read the comment docs.

@piksel
Copy link
Member

piksel commented Jun 9, 2021

It doesn't work with the Shoutrrr test app. My guess is that they stopped allowing it for newly created apps. I disabled it since it was officially not supposed to work and we cannot test it.

@evenh
Copy link
Author

evenh commented Jun 10, 2021

Huh, that doesn't match my experience (in practice). I have tested against an existing workspace while working on this PR.

For verification, I just created a new Slack workspace (shoutrrr-june-2021-repro/shoutrrrjune2-t6w4466.slack.com) where I added a new Incoming WebHooks integration (marked as legacy). This branch let's me invoke posting to custom channels and using an emoji. If you'd like I can send you an invite (whitelisted for @piksel.se) :-)

Default

./shoutrrr send -v --message=It works --url=slack://xxx/xxx/xxx
Screenshot 2021-06-10 at 09 57 32

Overridden emoji and channel name

./shoutrrr send -v --message=It works --url=slack://xxx/xxx/xxx?emoji=:sunny:&channel=debugging
Screenshot 2021-06-10 at 09 58 19

Overridden name, emoji and channel name

./shoutrrr send -v --message=It works --url=slack://my-bot-name@xxx/xxx/xxx?emoji=:sunny:&channel=debugging
Screenshot 2021-06-10 at 13 29 27

@piksel
Copy link
Member

piksel commented Jun 10, 2021

Ah, the legacy integration. That's probably why. I guess we could add a disclaimer that those options only works with the legacy webhooks.

@vsabella
Copy link

Would it be better to simply add support for the oauth bot token instead?

@piksel
Copy link
Member

piksel commented Jun 18, 2021

@vsabella Are you referring to apps' workspace bot user oauth tokens as mentioned in the app authentication basics?

That would indeed allow us to use the chat.postMessage endpoint instead which seems to allow all the necessary fields.
It could be reasonably easy to differentiate between the two types of tokens, since the webhook "id" is given in the Txxxx/Bxxxxx/xxxxx form and the token is xoxb-xxxx-xxx-xxxx.

We could use the API instead of the webhook when given a URL like:

slack://bot-user:xoxb-xxxx-xxx-xxxx@api?emoji=sunny&channel=bot-spam

My only concern is that it might introduce some confusion for the users ragarding how to set it up.

@piksel
Copy link
Member

piksel commented Jul 27, 2021

Superseded by #179

@piksel piksel closed this Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants