[BUG] Slack Engine cannot import slack client

[toabi]

Description
The slack engine fails to load with a recent version of the slack client library

Setup

• Salt v3001
• Configure a salt engine
• Install slackclient on the system (e.g. pip3 install slackclient)

Steps to Reproduce the behavior
When starting up and enabling debug logging, one can see this:

2020-06-30 07:46:46,163 [salt.utils.lazy :106 ][DEBUG ][30780] Could not LazyLoad slack.start: 'slack' __virtual__ returned False: The 'slackclient' Python module could not be loaded

https://github.com/saltstack/salt/blob/master/salt/engines/slack.py#L177

It's probably because slackclient is now different and the migration described in https://github.com/slackapi/python-slackclient/wiki/Migrating-to-2.x was not done.

Installing a v1 version fixed the import but yielded different errors inside the slack module.
Quickfixing the imports also didn't fully work.

Expected behavior

Salt engine starts with recent version of slackclient.

Versions Report

Salt Version:
           Salt: 3001
 
Dependency Versions:
           cffi: 1.13.2
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: 0.28.4
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: 3.6.1
         pygit2: 1.0.2
         Python: 3.7.3 (default, Dec 20 2019, 18:57:59)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 18.1.1
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.2
 
System Versions:
           dist: debian 10 buster
         locale: utf-8
        machine: x86_64
        release: 4.19.0-6-amd64
         system: Linux
        version: Debian GNU/Linux 10 buster

[cmcmarrow]

@toabi your right. Looks like the slack API did more than a naming convention change https://github.com/slackapi/python-slackclient/tree/master. sc.api_call(“chat.postMessage”… vs client.chat_postMessage(… But It looks like a simple restructure. So the good news is the module does not need a full rewrite but still needs some good work.
Does slack v1 work on an older version of salt? If not odds are slack changed their security protocols.

[toabi]

We actually have a Saltstack 3000.2 running with slackclient==1.0.0 still working. Funny.

Didn't manage to get the new setup to work with v1.3.2. I didn't dig much deeper though.

[cmcmarrow]

@toabi thanks for the information. I think the best course of action for this issue is to get 1.0.0 working on salt again. Then to upgrade our module to support the new API changes.

[lorenzoquantyca]

Do you have any news about the release date of this fix?

[sagetherage]

the Core team won't be able to get to this in Aluminium and moving directly to Silicon - next release cycle

[garethgreenaway]

Fixed by #60165

[ITJamie]

The above PR did not fix the code in the engine file. only states modules.

Response Text: {"ok":false,"error":"method_deprecated","response_metadata":{"messages":["[ERROR] This method is retired and can no longer be used. Please use conversations.list or users.conversations instead. Learn more: https:\/\/api.slack.com\/changelog\/2020-01-deprecating-antecedents-to-the-conversations-api."]}}
2021-09-10 10:14:03,661 [salt.engines     :132 ][CRITICAL][447636] Engine 'slack' could not be started!
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 936, in start
    client.run_commands_from_slack_async(message_generator, fire_all, tag, control)
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 780, in run_commands_from_slack_async
    for msg in message_generator:
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 508, in generate_triggered_messages
    all_slack_channels = self.get_slack_channels(
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 224, in get_slack_channels
    channels[item["id"]] = item["name"]
TypeError: string indices must be integers

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/engines/__init__.py", line 130, in run
    self.engine[self.fun](**kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 1241, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 2274, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader.py", line 2289, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 938, in start
    raise Exception("{}".format(traceback.format_exc()))
Exception: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 936, in start
    client.run_commands_from_slack_async(message_generator, fire_all, tag, control)
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 780, in run_commands_from_slack_async
    for msg in message_generator:
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 508, in generate_triggered_messages
    all_slack_channels = self.get_slack_channels(
  File "/usr/lib/python3/dist-packages/salt/engines/slack.py", line 224, in get_slack_channels
    channels[item["id"]] = item["name"]
TypeError: string indices must be integers

[OrangeDog]

get 1.0.0 working on salt again

It's too late for that now. Slack itself will no longer work with that API.
Migrating to the new API is the only option.

[ITJamie]

https://slack.dev/bolt-python - https://github.com/slackapi/bolt-python
Bolt seems to be the direction that slack are going for their newer apps.
I would hope this is the direction salt team chooses so that we are actually future proofed