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

Updating Slack engine to use slack_bolt #62957

Merged
merged 5 commits into from
Oct 31, 2022

Conversation

garethgreenaway
Copy link
Contributor

@garethgreenaway garethgreenaway commented Oct 25, 2022

What does this PR do?

Updating Slack engine to use slack_bolt

What issues does this PR fix or reference?

Fixes: #57842
Fixes: #62812

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@garethgreenaway garethgreenaway requested a review from a team as a code owner October 25, 2022 21:49
@garethgreenaway garethgreenaway requested review from whytewolf and removed request for a team October 25, 2022 21:49
@garethgreenaway garethgreenaway added the Sulfur v3006.0 release code name and version label Oct 26, 2022
MKLeb
MKLeb previously approved these changes Oct 26, 2022
twangboy
twangboy previously approved these changes Oct 27, 2022
MKLeb
MKLeb previously approved these changes Oct 27, 2022
whytewolf
whytewolf previously approved these changes Oct 28, 2022
@garethgreenaway garethgreenaway dismissed stale reviews from whytewolf, MKLeb, and twangboy via 126ed08 October 28, 2022 19:32
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks to be a breaking change for users using the slackclient as it will require them to install a dependency and update their configs to include app_token and bot_token. Is there a reason we are not including the old slackclient functionality and putting it on a deprecation path?

changelog/57842.fixed Show resolved Hide resolved
@Ch3LL Ch3LL merged commit ce425d2 into saltstack:master Oct 31, 2022
@garethgreenaway garethgreenaway deleted the slack_engine_slack_bolt branch October 31, 2022 19:05
@joshperry
Copy link

joshperry commented Nov 10, 2022

I think @Ch3LL is right here. The rtm API still works with slack and I just set up a "legacy bot user" which is able to successfully send messages from our v3004 install. It'd be nice to have a deprecation path for this so that our integration doesn't stop working immediately when we upgrade to v3006 with no way to smoothly transition.

It's also important to note that while Slack does recommend moving away from the rtm API, they have not deprecated it.

https://slack.dev/node-slack-sdk/rtm-api

RTM isn’t available for modern scoped apps anymore. We recommend using the Events API and Web API instead. If you need to use RTM (possibly due to corporate firewall limitations), you can do so by creating a legacy scoped app. If you have an existing RTM app, do not update its scopes as it will be updated to a modern scoped app and stop working with RTM.

Once you create a legacy scoped app, you just need to add the Bots feature, and when the app is installed you'll be given a bot token, which is what I put in the as the api key in our salt config.

image

On the permissions page for the app, there will be a warning recommending to update to granular scopes:
image

However, as the warning at the top of the scope update page says, the app will no longer be able to use the rtm API if you change to granular scopes:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sulfur v3006.0 release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Slack engine stopped working on September 28 [BUG] Slack Engine cannot import slack client
6 participants