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

Inconsistent argument naming in salt.modules.slack_notify.call_hook #58845

Open
Daniel-Lecoq opened this issue Oct 30, 2020 · 2 comments
Open
Labels
doc-param-missing parameters are missing in the docs doc-request net new docs requested Documentation Relates to Salt documentation severity-high 2nd top severity, seen by most users, causes major problems slack All salt stuff for slack time-estimate-sprint
Projects

Comments

@Daniel-Lecoq
Copy link

Description
To use the function call_hook in the module slack_notify you need to do the following

salt '*' slack.call_hook message='Hello, from SaltStack' identifier='T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX'

However, if you want to put the identifier in a configuration file eg. /etc/salt/minion.d/slack.conf the following wont work:

slack:
  identifier: T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX
sudo salt 'minion-1' slack.call_hook message='Hello, from SaltStack'
minion-1:
    ERROR executing 'slack.call_hook': No Slack WebHook url found

you need to write

slack:
  hook: T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX

This is not documented anywhere (I had to go look at the code for the module) and the error message isn't very helpful since it doesn't tell you what argument name it's looking for.

Suggested Fix
Simple fix: Update the documentation

Moderate fix: Update the documentation, error message and match the expected name in the configuration file with the expected name of the argument on the commandline (hook != identifier)

Outside scope: There are multiple returners and modules for Slack. If they could be synchronised so that we only need one configuration file for Slack that works across returners and modules that would simplify setup quite a bit.

Type of documentation
Salt modules

Location or format of documentation
https://docs.saltstack.com/en/3001/ref/modules/all/salt.modules.slack_notify.html

Additional context

    Salt Version:
               Salt: 3001.1

    Dependency Versions:
               cffi: Not Installed
           cherrypy: Not Installed
           dateutil: Not Installed
          docker-py: Not Installed
              gitdb: Not Installed
          gitpython: Not Installed
             Jinja2: 2.8.1
            libgit2: Not Installed
           M2Crypto: 0.33.0
               Mako: Not Installed
       msgpack-pure: Not Installed
     msgpack-python: 0.6.2
       mysql-python: Not Installed
          pycparser: Not Installed
           pycrypto: Not Installed
       pycryptodome: Not Installed
             pygit2: Not Installed
             Python: 3.6.8 (default, May  6 2020, 12:04:35)
       python-gnupg: Not Installed
             PyYAML: 3.11
              PyZMQ: 17.0.0
              smmap: Not Installed
            timelib: Not Installed
            Tornado: 4.5.3
                ZMQ: 4.1.4

    System Versions:
               dist: rhel 7.8 Maipo
             locale: UTF-8
            machine: x86_64
            release: 3.10.0-1127.19.1.el7.x86_64
             system: Linux
            version: Red Hat Enterprise Linux Server 7.8 Maipo
@Daniel-Lecoq Daniel-Lecoq added the Documentation Relates to Salt documentation label Oct 30, 2020
@welcome
Copy link

welcome bot commented Oct 30, 2020

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at core@saltstack.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@garethgreenaway garethgreenaway added severity-low 4th level, cosemtic problems, work around exists and removed needs-triage labels Jan 12, 2021
@garethgreenaway garethgreenaway added this to the Approved milestone Jan 12, 2021
@sagetherage sagetherage added this to To do in Salt Docs Mar 23, 2021
@sagetherage sagetherage added the slack All salt stuff for slack label May 7, 2021
@sagetherage sagetherage modified the milestones: Approved, Silicon May 7, 2021
@sagetherage sagetherage added the Silicon v3004.0 Release code name label May 7, 2021
@sagetherage sagetherage added doc-param-missing parameters are missing in the docs doc-request net new docs requested severity-high 2nd top severity, seen by most users, causes major problems Phosphorus v3005.0 Release code name and version and removed severity-low 4th level, cosemtic problems, work around exists Silicon v3004.0 Release code name labels Aug 3, 2021
@sagetherage sagetherage modified the milestones: Silicon, Phosphorus Aug 3, 2021
@sagetherage
Copy link
Contributor

updating this as a docs missing reference giving it a higher severity as the docs missing needs attention

@Ch3LL Ch3LL removed the Phosphorus v3005.0 Release code name and version label Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-param-missing parameters are missing in the docs doc-request net new docs requested Documentation Relates to Salt documentation severity-high 2nd top severity, seen by most users, causes major problems slack All salt stuff for slack time-estimate-sprint
Projects
Salt Docs
  
To do
Development

No branches or pull requests

8 participants