Postgres ssl mode prefer (#21498)

* refactor postgres,
* adds a basic unit test module
* first step towards a common utils module
* set postgresql_db doc argument defaults to what the code actually uses

* unit tests that actually test a missing/found psycopg2, no dependency needed

* add doc fragments, use common args, ansible2ify the imports

* update dict

* add AnsibleModule import

* mv AnsibleModule import to correct file

* restore some database utils we need

* rm some more duplicated pg doc fragments

* change ssl_mode from disable to prefer, add update docs

* use LibraryError pattern for import verification

per comments on #21435. basically LibraryError and touching up its usage in pg_db and the tests.

6 files changed, 256 insertions, 86 deletions

diff --git a/lib/ansible/module_utils/postgres.py b/lib/ansible/module_utils/postgres.py
new file mode 100644
index 0000000000..951b4fec5e
--- /dev/null
+++ b/lib/ansible/module_utils/postgres.py
@@ -0,0 +1,67 @@
+# This code is part of Ansible, but is an independent component.
+# This particular file snippet, and this file snippet only, is BSD licensed.
+# Modules you write using this snippet, which is embedded dynamically by Ansible
+# still belong to the author of the module, and may assign their own license
+# to the complete work.
+#
+# Copyright (c), Ted Timmons <ted@timmons.me>, 2017.
+# Most of this was originally added by other creators in the postgresql_user module.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without modification,
+# are permitted provided that the following conditions are met:
+#
+#    * Redistributions of source code must retain the above copyright
+#      notice, this list of conditions and the following disclaimer.
+#    * Redistributions in binary form must reproduce the above copyright notice,
+#      this list of conditions and the following disclaimer in the documentation
+#      and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+# IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+# USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# standard ansible imports
+from ansible.module_utils.basic import get_exception
+
+# standard PG imports
+HAS_PSYCOPG2 = False
+try:
+    import psycopg2
+    import psycopg2.extras
+except ImportError:
+    pass
+else:
+    HAS_PSYCOPG2 = True
+
+from ansible.module_utils.six import iteritems
+
+class LibraryError(Exception):
+    pass
+
+def ensure_libs(sslrootcert=None):
+    if not HAS_PSYCOPG2:
+        raise LibraryError('psycopg2 is not installed. we need psycopg2.')
+    if sslrootcert and psycopg2.__version__ < '2.4.3':
+        raise LibraryError('psycopg2 must be at least 2.4.3 in order to use the ssl_rootcert parameter')
+
+    # no problems
+    return None
+
+def postgres_common_argument_spec():
+    return dict(
+        login_user        = dict(default='postgres'),
+        login_password    = dict(default='', no_log=True),
+        login_host        = dict(default=''),
+        login_unix_socket = dict(default=''),
+        port              = dict(type='int', default=5432),
+        ssl_mode          = dict(default='prefer', choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
+        ssl_rootcert      = dict(default=None),
+    )
+
diff --git a/lib/ansible/modules/database/postgresql/postgresql_db.py b/lib/ansible/modules/database/postgresql/postgresql_db.py
index 3bfcb96107..54da6af165 100755
--- a/lib/ansible/modules/database/postgresql/postgresql_db.py
+++ b/lib/ansible/modules/database/postgresql/postgresql_db.py
@@ -33,36 +33,11 @@ options:
       - name of the database to add or remove
     required: true
     default: null
-  login_user:
-    description:
-      - The username used to authenticate with
-    required: false
-    default: null
-  login_password:
-    description:
-      - The password used to authenticate with
-    required: false
-    default: null
-  login_host:
-    description:
-      - Host running the database
-    required: false
-    default: localhost
-  login_unix_socket:
-    description:
-      - Path to a Unix domain socket for local connections
-    required: false
-    default: null
   owner:
     description:
       - Name of the role to set as owner of the database
     required: false
     default: null
-  port:
-    description:
-      - Database port to connect to.
-    required: false
-    default: 5432
   template:
     description:
       - Template used to create the database
@@ -89,27 +64,9 @@ options:
     required: false
     default: present
     choices: [ "present", "absent" ]
-  ssl_mode:
-    description:
-      - Determines whether or with what priority a secure SSL TCP/IP connection will be negotiated with the server.
-      - See https://www.postgresql.org/docs/current/static/libpq-ssl.html for more information on the modes.
-    required: false
-    default: disable
-    choices: [disable, allow, prefer, require, verify-ca, verify-full]
-    version_added: '2.3'
-  ssl_rootcert:
-    description:
-      - Specifies the name of a file containing SSL certificate authority (CA) certificate(s). If the file exists, the server's certificate will be verified to be signed by one of these authorities.
-    required: false
-    default: null
-    version_added: '2.3'
-notes:
-   - The default authentication assumes that you are either logging in as or sudo'ing to the C(postgres) account on the host.
-   - This module uses I(psycopg2), a Python PostgreSQL database adapter. You must ensure that psycopg2 is installed on
-     the host before using this module. If the remote host is the PostgreSQL server (which is the default case), then PostgreSQL must also be installed on the remote host. For Ubuntu-based systems, install the C(postgresql), C(libpq-dev), and C(python-psycopg2) packages on the remote host before using this module.
-   - The ssl_rootcert parameter requires at least Postgres version 8.4 and I(psycopg2) version 2.4.3.
-requirements: [ psycopg2 ]
 author: "Ansible Core Team"
+extends_documentation_fragment:
+- postgres
 '''
 
 EXAMPLES = '''
@@ -128,15 +85,23 @@ EXAMPLES = '''
     template: template0
 '''
 
+HAS_PSYCOPG2 = False
 try:
     import psycopg2
     import psycopg2.extras
 except ImportError:
-    postgresqldb_found = False
+    pass
 else:
-    postgresqldb_found = True
+    HAS_PSYCOPG2 = True
 from ansible.module_utils.six import iteritems
 
+import traceback
+
+import ansible.module_utils.postgres as pgutils
+from ansible.module_utils.database import SQLParseError, pg_quote_identifier
+from ansible.module_utils.basic import get_exception, AnsibleModule
+
+
 class NotSupportedError(Exception):
     pass
 
@@ -243,27 +208,24 @@ def db_matches(cursor, db, owner, template, encoding, lc_collate, lc_ctype):
 #
 
 def main():
+    argument_spec = pgutils.postgres_common_argument_spec()
+    argument_spec.update(dict(
+        db=dict(required=True, aliases=['name']),
+        owner=dict(default=""),
+        template=dict(default=""),
+        encoding=dict(default=""),
+        lc_collate=dict(default=""),
+        lc_ctype=dict(default=""),
+        state=dict(default="present", choices=["absent", "present"]),
+    ))
+
+
     module = AnsibleModule(
-        argument_spec=dict(
-            login_user=dict(default="postgres"),
-            login_password=dict(default="", no_log=True),
-            login_host=dict(default=""),
-            login_unix_socket=dict(default=""),
-            port=dict(default="5432"),
-            db=dict(required=True, aliases=['name']),
-            owner=dict(default=""),
-            template=dict(default=""),
-            encoding=dict(default=""),
-            lc_collate=dict(default=""),
-            lc_ctype=dict(default=""),
-            state=dict(default="present", choices=["absent", "present"]),
-            ssl_mode=dict(default="disable", choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
-            ssl_rootcert=dict(default=None),
-        ),
+        argument_spec=argument_spec,
         supports_check_mode = True
     )
 
-    if not postgresqldb_found:
+    if not HAS_PSYCOPG2:
         module.fail_json(msg="the python psycopg2 module is required")
 
     db = module.params["db"]
@@ -296,38 +258,36 @@ def main():
     if is_localhost and module.params["login_unix_socket"] != "":
         kw["host"] = module.params["login_unix_socket"]
 
-    if psycopg2.__version__ < '2.4.3' and sslrootcert is not None:
-        module.fail_json(msg='psycopg2 must be at least 2.4.3 in order to user the ssl_rootcert parameter')
-
     try:
+        pgutils.ensure_libs(sslrootcert=module.params.get('ssl_rootcert'))
         db_connection = psycopg2.connect(database="postgres", **kw)
         # Enable autocommit so we can create databases
         if psycopg2.__version__ >= '2.4.2':
             db_connection.autocommit = True
         else:
-            db_connection.set_isolation_level(psycopg2
-                                              .extensions
-                                              .ISOLATION_LEVEL_AUTOCOMMIT)
-        cursor = db_connection.cursor(
-            cursor_factory=psycopg2.extras.DictCursor)
+            db_connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
+        cursor = db_connection.cursor(cursor_factory=psycopg2.extras.DictCursor)
+
+    except pgutils.LibraryError:
+        e = get_exception()
+        module.fail_json(msg="unable to connect to database: {0}".format(str(e)), exception=traceback.format_exc())
 
     except TypeError:
         e = get_exception()
         if 'sslrootcert' in e.args[0]:
-            module.fail_json(msg='Postgresql server must be at least version 8.4 to support sslrootcert')
-        module.fail_json(msg="unable to connect to database: %s" % e)
+            module.fail_json(msg='Postgresql server must be at least version 8.4 to support sslrootcert. Exception: {0}'.format(e), exception=traceback.format_exc())
+        module.fail_json(msg="unable to connect to database: %s" % e, exception=traceback.format_exc())
 
     except Exception:
         e = get_exception()
-        module.fail_json(msg="unable to connect to database: %s" % e)
+        module.fail_json(msg="unable to connect to database: %s" % e, exception=traceback.format_exc())
 
     try:
         if module.check_mode:
             if state == "absent":
                 changed = db_exists(cursor, db)
             elif state == "present":
-                changed = not db_matches(cursor, db, owner, template, encoding,
-                                         lc_collate, lc_ctype)
+                changed = not db_matches(cursor, db, owner, template, encoding, lc_collate, lc_ctype)
             module.exit_json(changed=changed, db=db)
 
         if state == "absent":
@@ -339,8 +299,7 @@ def main():
 
         elif state == "present":
             try:
-                changed = db_create(cursor, db, owner, template, encoding,
-                                lc_collate, lc_ctype)
+                changed = db_create(cursor, db, owner, template, encoding, lc_collate, lc_ctype)
             except SQLParseError:
                 e = get_exception()
                 module.fail_json(msg=str(e))
@@ -356,8 +315,5 @@ def main():
 
     module.exit_json(changed=changed, db=db)
 
-# import module snippets
-from ansible.module_utils.basic import *
-from ansible.module_utils.database import *
 if __name__ == '__main__':
     main()
diff --git a/lib/ansible/modules/database/postgresql/postgresql_privs.py b/lib/ansible/modules/database/postgresql/postgresql_privs.py
index 323ddc55f6..5e82dfb8a3 100644
--- a/lib/ansible/modules/database/postgresql/postgresql_privs.py
+++ b/lib/ansible/modules/database/postgresql/postgresql_privs.py
@@ -124,8 +124,9 @@ options:
     description:
       - Determines whether or with what priority a secure SSL TCP/IP connection will be negotiated with the server.
       - See https://www.postgresql.org/docs/current/static/libpq-ssl.html for more information on the modes.
+      - Default of C(prefer) matches libpq default.
     required: false
-    default: disable
+    default: prefer
     choices: [disable, allow, prefer, require, verify-ca, verify-full]
     version_added: '2.3'
   ssl_rootcert:
@@ -571,7 +572,7 @@ def main():
             unix_socket=dict(default='', aliases=['login_unix_socket']),
             login=dict(default='postgres', aliases=['login_user']),
             password=dict(default='', aliases=['login_password'], no_log=True),
-            ssl_mode=dict(default="disable", choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
+            ssl_mode=dict(default="prefer", choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
             ssl_rootcert=dict(default=None)
         ),
         supports_check_mode = True
diff --git a/lib/ansible/modules/database/postgresql/postgresql_user.py b/lib/ansible/modules/database/postgresql/postgresql_user.py
index 13b6c6b9df..5071f6a27f 100644
--- a/lib/ansible/modules/database/postgresql/postgresql_user.py
+++ b/lib/ansible/modules/database/postgresql/postgresql_user.py
@@ -128,8 +128,9 @@ options:
     description:
       - Determines whether or with what priority a secure SSL TCP/IP connection will be negotiated with the server.
       - See https://www.postgresql.org/docs/current/static/libpq-ssl.html for more information on the modes.
+      - Default of C(prefer) matches libpq default.
     required: false
-    default: disable
+    default: prefer
     choices: [disable, allow, prefer, require, verify-ca, verify-full]
     version_added: '2.3'
   ssl_rootcert:
@@ -646,7 +647,7 @@ def main():
             encrypted=dict(type='bool', default='no'),
             no_password_changes=dict(type='bool', default='no'),
             expires=dict(default=None),
-            ssl_mode=dict(default='disable', choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
+            ssl_mode=dict(default='prefer', choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
             ssl_rootcert=dict(default=None)
         ),
         supports_check_mode = True
diff --git a/lib/ansible/utils/module_docs_fragments/postgres.py b/lib/ansible/utils/module_docs_fragments/postgres.py
new file mode 100644
index 0000000000..fee32607b5
--- /dev/null
+++ b/lib/ansible/utils/module_docs_fragments/postgres.py
@@ -0,0 +1,70 @@
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+
+class ModuleDocFragment(object):
+    # Postgres documentation fragment
+    DOCUMENTATION = """
+options:
+  login_user:
+    description:
+      - The username used to authenticate with
+    required: false
+    default: postgres
+  login_password:
+    description:
+      - The password used to authenticate with
+    required: false
+    default: null
+  login_host:
+    description:
+      - Host running the database
+    required: false
+    default: null
+  login_unix_socket:
+    description:
+      - Path to a Unix domain socket for local connections
+    required: false
+    default: null
+  port:
+    description:
+      - Database port to connect to.
+    required: false
+    default: 5432
+  ssl_mode:
+    description:
+      - Determines whether or with what priority a secure SSL TCP/IP connection will be negotiated with the server.
+      - See https://www.postgresql.org/docs/current/static/libpq-ssl.html for more information on the modes.
+      - Default of C(prefer) matches libpq default.
+    required: false
+    default: prefer
+    choices: [disable, allow, prefer, require, verify-ca, verify-full]
+    version_added: '2.3'
+  ssl_rootcert:
+    description:
+      - Specifies the name of a file containing SSL certificate authority (CA) certificate(s).
+      - If the file exists, the server's certificate will be verified to be signed by one of these authorities.
+    required: false
+    default: null
+    version_added: '2.3'
+notes:
+- The default authentication assumes that you are either logging in as or sudo'ing to the C(postgres) account on the host.
+- This module uses I(psycopg2), a Python PostgreSQL database adapter. You must ensure that psycopg2 is installed on
+  the host before using this module. If the remote host is the PostgreSQL server (which is the default case), then
+  PostgreSQL must also be installed on the remote host. For Ubuntu-based systems, install the C(postgresql), C(libpq-dev),
+  and C(python-psycopg2) packages on the remote host before using this module.
+- The ssl_rootcert parameter requires at least Postgres version 8.4 and I(psycopg2) version 2.4.3.
+requirements: [ psycopg2 ]
+"""
diff --git a/test/units/module_utils/test_postgresql.py b/test/units/module_utils/test_postgresql.py
new file mode 100644
index 0000000000..af6813f919
--- /dev/null
+++ b/test/units/module_utils/test_postgresql.py
@@ -0,0 +1,75 @@
+import json
+import sys
+
+from ansible.compat.tests import unittest
+from ansible.compat.tests.mock import patch, MagicMock
+from ansible.compat.six.moves import builtins
+
+from ansible.module_utils._text import to_native
+from ansible.module_utils.basic import AnsibleModule
+from units.mock.procenv import swap_stdin_and_argv
+
+
+import pprint
+
+realimport = builtins.__import__
+
+class TestPostgres(unittest.TestCase):
+    def clear_modules(self, mods):
+        for mod in mods:
+            if mod in sys.modules:
+                del sys.modules[mod]
+
+    @patch.object(builtins, '__import__')
+    def test_postgres_pg2_missing_ensure_libs(self, mock_import):
+        def _mock_import(name, *args, **kwargs):
+            if name == 'psycopg2':
+                raise ImportError
+            return realimport(name, *args, **kwargs)
+
+        self.clear_modules(['psycopg2', 'ansible.module_utils.postgres'])
+        mock_import.side_effect = _mock_import
+        mod = builtins.__import__('ansible.module_utils.postgres')
+
+        self.assertFalse(mod.module_utils.postgres.HAS_PSYCOPG2)
+
+        with self.assertRaises(mod.module_utils.postgres.LibraryError) as context:
+            mod.module_utils.postgres.ensure_libs(sslrootcert=None)
+        self.assertIn('psycopg2 is not installed', to_native(context.exception))
+
+    @patch.object(builtins, '__import__')
+    def test_postgres_pg2_found_ensure_libs(self, mock_import):
+        def _mock_import(name, *args, **kwargs):
+            if 'psycopg2' in name:
+                return MagicMock()
+            return realimport(name, *args, **kwargs)
+
+        self.clear_modules(['psycopg2', 'ansible.module_utils.postgres'])
+        mock_import.side_effect = _mock_import
+        mod = builtins.__import__('ansible.module_utils.postgres')
+
+        self.assertTrue(mod.module_utils.postgres.HAS_PSYCOPG2)
+
+        ensure_ret = mod.module_utils.postgres.ensure_libs(sslrootcert=None)
+        self.assertFalse(ensure_ret)
+        pprint.pprint(ensure_ret)
+
+    @patch.object(builtins, '__import__')
+    def test_postgres_pg2_found_ensure_libs_old_version(self, mock_import):
+        def _mock_import(name, *args, **kwargs):
+            if 'psycopg2' in name:
+                m = MagicMock()
+                m.__version__ = '2.4.1'
+                return m
+            return realimport(name, *args, **kwargs)
+
+        self.clear_modules(['psycopg2', 'ansible.module_utils.postgres'])
+        mock_import.side_effect = _mock_import
+        mod = builtins.__import__('ansible.module_utils.postgres')
+
+        self.assertTrue(mod.module_utils.postgres.HAS_PSYCOPG2)
+
+        with self.assertRaises(mod.module_utils.postgres.LibraryError) as context:
+            mod.module_utils.postgres.ensure_libs(sslrootcert='yes')
+        self.assertIn('psycopg2 must be at least 2.4.3 in order to use', to_native(context.exception))
+