mongodb_user.present returns successfully on error

[mamalos]

Description of Issue

When trying to add a user using mongodb.user_present on a Mongo 4.0.7 server, even though /var/log/salt/minion errs with: "Creating database admin failed with error: Use of SCRAM-SHA-256 requires undigested passwords".

As far as I understand, the master should be informed about the error (and all other errors that are probably hidden) and return a failed state.

Using the latest version of pymongo (using pip) solves the problem of the user not being created, but this, obviously, doesn't solve the non failed state problem.

Setup

Minion installed in debian-9 on a SmartOS lx zone (version 20180404), python version 2.7.13-2, salt-minion version 2019.2.0 (Fluorine). salt-master version 2018.3.1 (Oxygen, running on SmartOS zone)

Steps to Reproduce Issue

run the following state on the minion:

create admin user:
  mongodb_user.present:
    - name: admin
    - passwd: mypass
    - host: 192.168.150.70
    - roles:
      - readWriteAnyDatabase

output on master:

minionid:
----------
          ID: create admin user
    Function: mongodb_user.present
        Name: admin
      Result: True
     Comment: User admin is already present
     Started: 12:09:17.771806
    Duration: 43.716 ms
     Changes:   

Summary for minion
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:  43.716 ms

logs in /var/log/salt/minion:
2019-06-05 12:09:17,812 [salt.loaded.int.module.mongodb:294 ][ERROR ][83009] Creating database admin failed with error: Use of SCRAM-SHA-256 requires undigested passwords

Versions Report

minion:

# salt-minion --versions-report
/usr/lib/python2.7/dist-packages/salt/scripts.py:198: DeprecationWarning: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date.  Salt will drop support for Python 2.7 in the Sodium release or later.
Salt Version:
           Salt: 2019.2.0
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9.4 
         locale: UTF-8
        machine: x86_64
        release: 3.16.0
         system: Linux
        version: debian 9.4 

master:

# salt-master --version
Salt Version:
           Salt: 2018.3.1
 
Dependency Versions:
           cffi: 1.11.5
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.18
       pycrypto: 3.6.3
   pycryptodome: 3.6.3
         pygit2: Not Installed
         Python: 2.7.15 (default, Jul  3 2018, 07:38:19)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 17.0.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.3
 
System Versions:
           dist:   
         locale: 646
        machine: i86pc
        release: 5.11
         system: SunOS
        version: Not Installed

If a user connects to the mongo server using the command-line mongo client, they can verify that the user has not been created in the admin database.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[root360-StefanHeitmueller]

    try:
        log.info("Creating user %s", name)
        mdb = pymongo.database.Database(conn, database)
        mdb.add_user(name, passwd, roles=roles)
    except pymongo.errors.PyMongoError as err:
        log.error("Creating database %s failed with error: %s", name, err)
        return six.text_type(err)

https://github.com/saltstack/salt/blob/v3001.7/salt/modules/mongodb.py#L307-L313

Where return value is u'Use of SCRAM-SHA-256 requires undigested passwords' (which is True).

And then https://github.com/saltstack/salt/blob/master/salt/states/mongodb_user.py#L162-L177

    if __salt__["mongodb.user_create"](
        name,
        passwd,
        user,
        password,
        host,
        port,
        database=database,
        authdb=authdb,
        roles=roles,
    ):
        ret["comment"] = "User {0} has been created".format(name)
        ret["changes"][name] = "Present"
    else:
        ret["comment"] = "Failed to create database {0}".format(name)
        ret["result"] = False

will never enter the else, as the returned string is true.

As the error is being logged already, it might be sufficient to just return False instead of the error message.