postgresql_user: fix bugs related to 'expires' option (#23862)

* Factorize tests related to no_password_change using an include task

* Refactor: deduplicate tasks

* postgresql_user: test 'expires' parameter

* Change 'valid until' even it's the only updated field

* value is changed when another value is provided

* value isn't returned when unset

* Remove unused variable

* psycopg2.extras.DictRow is able to handle comparison

* postgresql_user: simplify helper method

* postgresql_user: define variable just before using it

* Fix comparison between user input and applied configuration

* new test: adding an invalid attribute

* Refactor, add cleaning task

* Check that using same attribute a 2nd time does nothing

* Always try to remove created user

* postgresql_user: fix pep8

lib/ansible/modules/database/postgresql/postgresql_user.py

@@ -207,11 +207,10 @@ EXAMPLES = '''
     password: NULL
 '''
 
-from hashlib import md5
 import itertools
 import re
-
 from distutils.version import StrictVersion
+from hashlib import md5
 
 try:
     import psycopg2
@@ -220,15 +219,15 @@ except ImportError:
     postgresqldb_found = False
 else:
     postgresqldb_found = True
+
 from ansible.module_utils._text import to_bytes
 from ansible.module_utils.six import iteritems
 
-_flags = ('SUPERUSER', 'CREATEROLE', 'CREATEUSER', 'CREATEDB', 'INHERIT', 'LOGIN', 'REPLICATION')
+FLAGS = ('SUPERUSER', 'CREATEROLE', 'CREATEUSER', 'CREATEDB', 'INHERIT', 'LOGIN', 'REPLICATION')
-_flags_by_version = {'BYPASSRLS': '9.5.0'}
+FLAGS_BY_VERSION = {'BYPASSRLS': '9.5.0'}
 
 VALID_PRIVS = dict(table=frozenset(('SELECT', 'INSERT', 'UPDATE', 'DELETE', 'TRUNCATE', 'REFERENCES', 'TRIGGER', 'ALL')),
-        database=frozenset(('CREATE', 'CONNECT', 'TEMPORARY', 'TEMP', 'ALL')),
+                   database=frozenset(('CREATE', 'CONNECT', 'TEMPORARY', 'TEMP', 'ALL')),)
-        )
 
 # map to cope with idiosyncracies of SUPERUSER and LOGIN
 PRIV_TO_AUTHID_COLUMN = dict(SUPERUSER='rolsuper', CREATEROLE='rolcreaterole',
@@ -236,9 +235,11 @@ PRIV_TO_AUTHID_COLUMN = dict(SUPERUSER='rolsuper', CREATEROLE='rolcreaterole',
                              INHERIT='rolinherit', LOGIN='rolcanlogin',
                              REPLICATION='rolreplication', BYPASSRLS='rolbypassrls')
 
+
 class InvalidFlagsError(Exception):
     pass
 
+
 class InvalidPrivsError(Exception):
     pass
 
@@ -260,9 +261,9 @@ def user_add(cursor, user, password, role_attr_flags, encrypted, expires):
     """Create a new database user (role)."""
     # Note: role_attr_flags escaped by parse_role_attrs and encrypted is a literal
     query_password_data = dict(password=password, expires=expires)
-    query = ['CREATE USER %(user)s' % { "user": pg_quote_identifier(user, 'role')}]
+    query = ['CREATE USER %(user)s' % {"user": pg_quote_identifier(user, 'role')}]
     if password is not None:
-        query.append("WITH %(crypt)s" % { "crypt": encrypted })
+        query.append("WITH %(crypt)s" % {"crypt": encrypted})
         query.append("PASSWORD %(password)s")
     if expires is not None:
         query.append("VALID UNTIL %(expires)s")
@@ -271,6 +272,7 @@ def user_add(cursor, user, password, role_attr_flags, encrypted, expires):
     cursor.execute(query, query_password_data)
     return True
 
+
 def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expires, no_password_changes):
     """Change user password and/or attributes. Return True if changed, False otherwise."""
     changed = False
@@ -285,9 +287,8 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir
             return False
 
     # Handle passwords.
-    if not no_password_changes and (password is not None or role_attr_flags != ''):
+    if not no_password_changes and (password is not None or role_attr_flags != '' or expires is not None):
         # Select password and all flag-like columns in order to verify changes.
-        query_password_data = dict(password=password, expires=expires)
         select = "SELECT * FROM pg_authid where rolname=%(user)s"
         cursor.execute(select, {"user": user})
         # Grab current role attributes.
@@ -300,7 +301,7 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir
             #  3: The size of the 'md5' prefix
             # When the provided password looks like a MD5-hash, value of
             # 'encrypted' is ignored.
-            if ((password.startswith('md5') and len(password) == 32+3) or encrypted == 'UNENCRYPTED'):
+            if ((password.startswith('md5') and len(password) == 32 + 3) or encrypted == 'UNENCRYPTED'):
                 if password != current_role_attrs['rolpassword']:
                     pwchanging = True
             elif encrypted == 'ENCRYPTED':
@@ -321,7 +322,12 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir
                 if current_role_attrs[PRIV_TO_AUTHID_COLUMN[role_attr_name]] != role_attr_value:
                     role_attr_flags_changing = True
 
-        expires_changing = (expires is not None and expires == current_role_attrs['rolvaliduntil'])
+        if expires is not None:
+            cursor.execute("SELECT %s::timestamptz;", (expires,))
+            expires_with_tz = cursor.fetchone()[0]
+            expires_changing = expires_with_tz != current_role_attrs.get('rolvaliduntil')
+        else:
+            expires_changing = False
 
         if not pwchanging and not role_attr_flags_changing and not expires_changing:
             return False
@@ -336,6 +342,7 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir
         if expires is not None:
             alter.append("VALID UNTIL %(expires)s")
 
+        query_password_data = dict(password=password, expires=expires)
         try:
             cursor.execute(' '.join(alter), query_password_data)
             changed = True
@@ -396,12 +403,11 @@ def user_alter(cursor, module, user, password, role_attr_flags, encrypted, expir
         new_role_attrs = cursor.fetchone()
 
         # Detect any differences between current_ and new_role_attrs.
-        for i in range(len(current_role_attrs)):
+        changed = current_role_attrs != new_role_attrs
-            if current_role_attrs[i] != new_role_attrs[i]:
-                changed = True
 
     return changed
 
+
 def user_delete(cursor, user):
     """Try to remove a user. Returns True if successful otherwise False"""
     cursor.execute("SAVEPOINT ansible_pgsql_user_delete")
@@ -415,6 +421,7 @@ def user_delete(cursor, user):
     cursor.execute("RELEASE SAVEPOINT ansible_pgsql_user_delete")
     return True
 
+
 def has_table_privileges(cursor, user, table, privs):
     """
     Return the difference between the privileges that a user already has and
@@ -431,6 +438,7 @@ def has_table_privileges(cursor, user, table, privs):
     desired = privs.difference(cur_privs)
     return (have_currently, other_current, desired)
 
+
 def get_table_privileges(cursor, user, table):
     if '.' in table:
         schema, table = table.split('.', 1)
@@ -441,25 +449,28 @@ def get_table_privileges(cursor, user, table):
     cursor.execute(query, (user, table, schema))
     return frozenset([x[0] for x in cursor.fetchall()])
 
+
 def grant_table_privileges(cursor, user, table, privs):
     # Note: priv escaped by parse_privs
     privs = ', '.join(privs)
     query = 'GRANT %s ON TABLE %s TO %s' % (
-        privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') )
+        privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role'))
     cursor.execute(query)
 
+
 def revoke_table_privileges(cursor, user, table, privs):
     # Note: priv escaped by parse_privs
     privs = ', '.join(privs)
     query = 'REVOKE %s ON TABLE %s FROM %s' % (
-        privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role') )
+        privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role'))
     cursor.execute(query)
 
+
 def get_database_privileges(cursor, user, db):
     priv_map = {
-        'C':'CREATE',
+        'C': 'CREATE',
-        'T':'TEMPORARY',
+        'T': 'TEMPORARY',
-        'c':'CONNECT',
+        'c': 'CONNECT',
     }
     query = 'SELECT datacl FROM pg_database WHERE datname = %s'
     cursor.execute(query, (db,))
@@ -474,6 +485,7 @@ def get_database_privileges(cursor, user, db):
         o.add(priv_map[v])
     return normalize_privileges(o, 'database')
 
+
 def has_database_privileges(cursor, user, db, privs):
     """
     Return the difference between the privileges that a user already has and
@@ -490,9 +502,10 @@ def has_database_privileges(cursor, user, db, privs):
     desired = privs.difference(cur_privs)
     return (have_currently, other_current, desired)
 
+
 def grant_database_privileges(cursor, user, db, privs):
     # Note: priv escaped by parse_privs
-    privs =', '.join(privs)
+    privs = ', '.join(privs)
     if user == "PUBLIC":
         query = 'GRANT %s ON DATABASE %s TO PUBLIC' % (
                 privs, pg_quote_identifier(db, 'database'))
@@ -502,6 +515,7 @@ def grant_database_privileges(cursor, user, db, privs):
                 pg_quote_identifier(user, 'role'))
     cursor.execute(query)
 
+
 def revoke_database_privileges(cursor, user, db, privs):
     # Note: priv escaped by parse_privs
     privs = ', '.join(privs)
@@ -514,6 +528,7 @@ def revoke_database_privileges(cursor, user, db, privs):
                 pg_quote_identifier(user, 'role'))
     cursor.execute(query)
 
+
 def revoke_privileges(cursor, user, privs):
     if privs is None:
         return False
@@ -532,6 +547,7 @@ def revoke_privileges(cursor, user, privs):
                 changed = True
     return changed
 
+
 def grant_privileges(cursor, user, privs):
     if privs is None:
         return False
@@ -550,6 +566,7 @@ def grant_privileges(cursor, user, privs):
                 changed = True
     return changed
 
+
 def parse_role_attrs(cursor, role_attr_flags):
     """
     Parse role attributes string for user creation.
@@ -567,20 +584,17 @@ def parse_role_attrs(cursor, role_attr_flags):
     Note: "[NO]BYPASSRLS" role attribute introduced in 9.5
 
     """
-    flags = frozenset(itertools.chain(_flags, get_valid_flags_by_version(cursor)))
+    flags = frozenset(role.upper() for role in role_attr_flags.split(',') if role)
-    valid_flags = frozenset(itertools.chain(flags, ('NO%s' % f for f in flags)))
 
-    if ',' in role_attr_flags:
+    valid_flags = frozenset(itertools.chain(FLAGS, get_valid_flags_by_version(cursor)))
-        flag_set = frozenset(r.upper() for r in role_attr_flags.split(","))
+    valid_flags = frozenset(itertools.chain(valid_flags, ('NO%s' % flag for flag in valid_flags)))
-    elif role_attr_flags:
+
-        flag_set = frozenset((role_attr_flags.upper(),))
+    if not flags.issubset(valid_flags):
-    else:
-        flag_set = frozenset()
-    if not flag_set.issubset(valid_flags):
         raise InvalidFlagsError('Invalid role_attr_flags specified: %s' %
-                ' '.join(flag_set.difference(valid_flags)))
+                                ' '.join(flags.difference(valid_flags)))
-    o_flags = ' '.join(flag_set)
+
-    return o_flags
+    return ' '.join(flags)
+
 
 def normalize_privileges(privs, type_):
     new_privs = set(privs)
@@ -593,6 +607,7 @@ def normalize_privileges(privs, type_):
 
     return new_privs
 
+
 def parse_privs(privs, db):
     """
     Parse privilege string to determine permissions for database db.
@@ -609,8 +624,8 @@ def parse_privs(privs, db):
         return privs
 
     o_privs = {
-        'database':{},
+        'database': {},
-        'table':{}
+        'table': {}
     }
     for token in privs.split('/'):
         if ':' not in token:
@@ -624,13 +639,14 @@ def parse_privs(privs, db):
 
         if not priv_set.issubset(VALID_PRIVS[type_]):
             raise InvalidPrivsError('Invalid privs specified for %s: %s' %
-                (type_, ' '.join(priv_set.difference(VALID_PRIVS[type_]))))
+                                    (type_, ' '.join(priv_set.difference(VALID_PRIVS[type_]))))
 
         priv_set = normalize_privileges(priv_set, type_)
         o_privs[type_][name] = priv_set
 
     return o_privs
 
+
 def get_pg_server_version(cursor):
     """
     Queries Postgres for its server version.
@@ -646,6 +662,7 @@ def get_pg_server_version(cursor):
     cursor.execute("SHOW SERVER_VERSION")
     return cursor.fetchone()['server_version']
 
+
 def get_valid_flags_by_version(cursor):
     """
     Some role attributes were introduced after certain versions. We want to
@@ -655,7 +672,7 @@ def get_valid_flags_by_version(cursor):
 
     return [
         flag
-        for flag, version_introduced in _flags_by_version.items()
+        for flag, version_introduced in FLAGS_BY_VERSION.items()
         if current_version >= StrictVersion(version_introduced)
     ]
 
@@ -685,7 +702,7 @@ def main():
             ssl_mode=dict(default='prefer', choices=['disable', 'allow', 'prefer', 'require', 'verify-ca', 'verify-full']),
             ssl_rootcert=dict(default=None)
         ),
-        supports_check_mode = True
+        supports_check_mode=True
     )
 
     user = module.params["user"]
@@ -696,7 +713,6 @@ def main():
     if db == '' and module.params["priv"] is not None:
         module.fail_json(msg="privileges require a database to be specified")
     privs = parse_privs(module.params["priv"], db)
-    port = module.params["port"]
     no_password_changes = module.params["no_password_changes"]
     if module.params["encrypted"]:
         encrypted = "ENCRYPTED"
@@ -712,15 +728,15 @@ def main():
     # check which values are empty and don't include in the **kw
     # dictionary
     params_map = {
-        "login_host":"host",
+        "login_host": "host",
-        "login_user":"user",
+        "login_user": "user",
-        "login_password":"password",
+        "login_password": "password",
-        "port":"port",
+        "port": "port",
-        "db":"database",
+        "db": "database",
-        "ssl_mode":"sslmode",
+        "ssl_mode": "sslmode",
-        "ssl_rootcert":"sslrootcert"
+        "ssl_rootcert": "sslrootcert"
     }
-    kw = dict( (params_map[k], v) for (k, v) in iteritems(module.params)
+    kw = dict((params_map[k], v) for (k, v) in iteritems(module.params)
               if k in params_map and v != "" and v is not None)
 
     # If a login_unix_socket is specified, incorporate it here.
@@ -800,6 +816,7 @@ def main():
     kw['changed'] = changed
     module.exit_json(**kw)
 
+
 # import module snippets
 from ansible.module_utils.basic import *
 from ansible.module_utils.database import *

test/integration/targets/postgresql/tasks/main.yml

@@ -184,9 +184,9 @@
       - "result.stdout_lines[-1] == '(0 rows)'"
 
 #
-# Create and destroy user
+# Create and destroy user, test 'password' and 'encrypted' parameters
 #
-- include: test_user.yml
+- include: test_password.yml
   vars:
     encrypted: '{{ item.user_creation_encrypted_value }}'
     db_password1: 'secretù' # use UTF-8
@@ -194,154 +194,31 @@
     - user_creation_encrypted_value: 'yes'
     - user_creation_encrypted_value: 'no'
 
-# BYPASSRLS role attribute was introduced in Postgres 9.5, so 
+# BYPASSRLS role attribute was introduced in PostgreSQL 9.5, so
 # we want to test atrribute management differently depending
-# on the version. See https://github.com/ansible/ansible/pull/24625
+# on the version.
-# for more details.
+- name: Get PostgreSQL version
-- name: Get Postgres version 
   become_user: "{{ pg_user }}"
   become: True
-  shell: echo 'SHOW SERVER_VERSION' | psql -d postgres
+  shell: "echo 'SHOW SERVER_VERSION' | psql --tuples-only --no-align --dbname postgres"
   register: postgres_version_resp
 
-- name: Print Postgres server version
+- name: Print PostgreSQL server version
   debug:
-    msg: "{{ postgres_version_resp.stdout_lines[-2] | trim }}"
+    msg: "{{ postgres_version_resp.stdout }}"
 
-- name: Role attribute testing for Postgres 9.5+
+- set_fact:
-  include: postgresql_user_9.5_or_greater.yml
+    bypassrls_supported: "{{ postgres_version_resp.stdout | version_compare('9.5.0', '>=') }}"
-  when: (postgres_version_resp.stdout_lines[-2] | trim) | version_compare('9.5.0', '>=')
 
-- name: Role attribute testing for Postgres versions below 9.5 
+# test 'no_password_change' and 'role_attr_flags' parameters
-  include: postgresql_user_less_than_9.5.yml
+- include: test_no_password_change.yml
-  when: (postgres_version_resp.stdout_lines[-2] | trim) | version_compare('9.5.0', '<')
+  vars:
-
+    no_password_changes: '{{ item }}'
-- name: Cleanup the user
+  with_items:
-  become_user: "{{ pg_user }}"
+    - 'yes'
-  become: True
+    - 'no'
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: 'absent'
-    login_user: "{{ pg_user }}"
-    db: postgres
-
-- name: Check that they were removed
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select * from pg_user where usename='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(0 rows)'"
-
-# Test cases to replicate issue 19835
-- name: Create a user "{{ db_user3 }}"  to test issue 19835
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user3 }}"
-    encrypted: 'yes'
-    password: "md55c8ccfd9d6711fc69a7eae647fc54f51"
-    login_user: "{{ pg_user }}"
-    #role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports that "{{ db_user3 }}" was created for testing issue 19835
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: debug result
-  debug:
-    var: result
-
-- name: Check that "{{ db_user3 }}" was created for testing issue 19835
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select * from pg_user where usename='{{ db_user3 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-
-- name: Modify user "{{ db_user3 }}" to have only login role attributes for testing issue 19835
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user3 }}"
-    state: "present"
-    role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit"
-    login_user: "{{ pg_user }}"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports it modified the roles for testing issue 19835
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: Check that the user "{{ db_user3 }}" has the requested role attributes for testing issue 19835
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user3 }}';" | psql -d postgres
-  register: result
-
-- name: Modify a single role attribute on the user "{{ db_user3 }}"  with no_password_changes set to yes. issue 19835
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user3 }}"
-    state: "present"
-    role_attr_flags: "CREATEDB"
-    no_password_changes: yes
-    login_user: "{{ pg_user }}"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports it modified the role with no_password_changes set to yes. issue 19835
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: Check that the user "{{ db_user3 }}" has the requested role attributes with no_password_changes set to yes. issue 19835
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin  from pg_roles where rolname='{{ db_user3 }}';" | psql -d postgres
-  register: result
-
-- name: Assert that the request role attributes check for user "{{ db_user3 }}" was correct with no_password_changes set to yes. issue 19835
-  assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:f' in result.stdout_lines[-2]"
-      - "'createrole:f' in result.stdout_lines[-2]"
-      - "'create:t' in result.stdout_lines[-2]"
-      - "'inherit:f' in result.stdout_lines[-2]"
-      - "'login:t' in result.stdout_lines[-2]"
-
-- name: Cleanup the "{{ db_user3 }}" user
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user3 }}"
-    state: 'absent'
-    login_user: "{{ pg_user }}"
-    db: postgres
-
-- name: Check that "{{ db_user3 }}"  was removed
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select * from pg_user where usename='{{ db_user3 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(0 rows)'"
 
-### TODO: test expires, fail_on_user
+### TODO: fail_on_user
 
 #
 # Test db ownership

test/integration/targets/postgresql/tasks/postgresql_user_9.5_or_greater.yml

@@ -1,90 +0,0 @@
----
-- name: Create a user with all role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: "present"
-    role_attr_flags: "SUPERUSER,CREATEROLE,CREATEDB,INHERIT,login,BYPASSRLS"
-    login_user: "{{ pg_user }}"
-    db: postgres
-
-- name: Check that the user has the requested role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin, 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:t' in result.stdout_lines[-2]"
-      - "'createrole:t' in result.stdout_lines[-2]"
-      - "'create:t' in result.stdout_lines[-2]"
-      - "'inherit:t' in result.stdout_lines[-2]"
-      - "'login:t' in result.stdout_lines[-2]"
-      - "'bypassrls:t' in result.stdout_lines[-2]"
-
-- name: Modify a user to have no role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: "present"
-    role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN,NOBYPASSRLS"
-    login_user: "{{ pg_user }}"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports it modified the role
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: Check that the user has the requested role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin, 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:f' in result.stdout_lines[-2]"
-      - "'createrole:f' in result.stdout_lines[-2]"
-      - "'create:f' in result.stdout_lines[-2]"
-      - "'inherit:f' in result.stdout_lines[-2]"
-      - "'login:f' in result.stdout_lines[-2]"
-      - "'bypassrls:f' in result.stdout_lines[-2]"
-
-- name: Modify a single role attribute on a user
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: "present"
-    role_attr_flags: "LOGIN"
-    login_user: "{{ pg_user }}"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports it modified the role
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: Check that the user has the requested role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin, 'bypassrls:'||rolbypassrls  from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:f' in result.stdout_lines[-2]"
-      - "'createrole:f' in result.stdout_lines[-2]"
-      - "'create:f' in result.stdout_lines[-2]"
-      - "'inherit:f' in result.stdout_lines[-2]"
-      - "'login:t' in result.stdout_lines[-2]"
-      - "'bypassrls:f' in result.stdout_lines[-2]"

test/integration/targets/postgresql/tasks/postgresql_user_less_than_9.5.yml

@@ -1,87 +0,0 @@
----
-- name: Create a user with all role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: "present"
-    role_attr_flags: "SUPERUSER,CREATEROLE,CREATEDB,INHERIT,login"
-    login_user: "{{ pg_user }}"
-    db: postgres
-
-- name: Check that the user has the requested role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:t' in result.stdout_lines[-2]"
-      - "'createrole:t' in result.stdout_lines[-2]"
-      - "'create:t' in result.stdout_lines[-2]"
-      - "'inherit:t' in result.stdout_lines[-2]"
-      - "'login:t' in result.stdout_lines[-2]"
-
-- name: Modify a user to have no role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: "present"
-    role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN"
-    login_user: "{{ pg_user }}"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports it modified the role
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: Check that the user has the requested role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:f' in result.stdout_lines[-2]"
-      - "'createrole:f' in result.stdout_lines[-2]"
-      - "'create:f' in result.stdout_lines[-2]"
-      - "'inherit:f' in result.stdout_lines[-2]"
-      - "'login:f' in result.stdout_lines[-2]"
-
-- name: Modify a single role attribute on a user
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_user:
-    name: "{{ db_user1 }}"
-    state: "present"
-    role_attr_flags: "LOGIN"
-    login_user: "{{ pg_user }}"
-    db: postgres
-  register: result
-
-- name: Check that ansible reports it modified the role
-  assert:
-    that:
-      - "result.changed == True"
-
-- name: Check that the user has the requested role attributes
-  become_user: "{{ pg_user }}"
-  become: True
-  shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin  from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
-  register: result
-
-- assert:
-    that:
-      - "result.stdout_lines[-1] == '(1 row)'"
-      - "'super:f' in result.stdout_lines[-2]"
-      - "'createrole:f' in result.stdout_lines[-2]"
-      - "'create:f' in result.stdout_lines[-2]"
-      - "'inherit:f' in result.stdout_lines[-2]"
-      - "'login:t' in result.stdout_lines[-2]"

test/integration/targets/postgresql/tasks/test_no_password_change.yml

@@ -0,0 +1,167 @@
+- vars:
+    task_parameters: &task_parameters
+      become_user: "{{ pg_user }}"
+      become: True
+      register: result
+    postgresql_parameters: &parameters
+      db: postgres
+      name: "{{ db_user1 }}"
+      login_user: "{{ pg_user }}"
+
+  block:
+
+  - name: Create a user with all role attributes
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: "present"
+      role_attr_flags: "SUPERUSER,CREATEROLE,CREATEDB,INHERIT,login{{ bypassrls_supported | ternary(',BYPASSRLS', '') }}"
+      no_password_changes: '{{ no_password_changes }}' # no_password_changes is ignored when user doesn't already exist
+
+  - name: Check that the user has the requested role attributes
+    <<: *task_parameters
+    shell: "echo \"select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin {{ bypassrls_supported | ternary(\", 'bypassrls:'||rolbypassrls\", '') }} from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres"
+
+  - assert:
+      that:
+        - "result.stdout_lines[-1] == '(1 row)'"
+        - "'super:t' in result.stdout_lines[-2]"
+        - "'createrole:t' in result.stdout_lines[-2]"
+        - "'create:t' in result.stdout_lines[-2]"
+        - "'inherit:t' in result.stdout_lines[-2]"
+        - "'login:t' in result.stdout_lines[-2]"
+
+  - block:
+      - name: Check that the user has the requested role attribute BYPASSRLS
+        <<: *task_parameters
+        shell: "echo \"select 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres"
+
+      - assert:
+          that:
+            - "not bypassrls_supported or 'bypassrls:t' in result.stdout_lines[-2]"
+    when: bypassrls_supported
+
+  - name: Modify a user to have no role attributes
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: "present"
+      role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN{{ bypassrls_supported | ternary(',NOBYPASSRLS', '') }}"
+      no_password_changes: '{{ no_password_changes }}'
+
+  - name: Check that ansible reports it modified the role
+    assert:
+      that:
+        - "result.changed"
+
+  - name: "Check that the user doesn't have any attribute"
+    <<: *task_parameters
+    shell: "echo \"select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres"
+
+  - assert:
+      that:
+        - "result.stdout_lines[-1] == '(1 row)'"
+        - "'super:f' in result.stdout_lines[-2]"
+        - "'createrole:f' in result.stdout_lines[-2]"
+        - "'create:f' in result.stdout_lines[-2]"
+        - "'inherit:f' in result.stdout_lines[-2]"
+        - "'login:f' in result.stdout_lines[-2]"
+
+  - block:
+      - name: Check that the user has the requested role attribute BYPASSRLS
+        <<: *task_parameters
+        shell: "echo \"select 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';\" | psql -d postgres"
+
+      - assert:
+          that:
+            - "not bypassrls_supported or 'bypassrls:f' in result.stdout_lines[-2]"
+    when: bypassrls_supported
+
+  - name: Try to add an invalid attribute
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: "present"
+      role_attr_flags: "NOSUPERUSER,NOCREATEROLE,NOCREATEDB,noinherit,NOLOGIN{{ bypassrls_supported | ternary(',NOBYPASSRLS', '') }},INVALID"
+      no_password_changes: '{{ no_password_changes }}'
+    ignore_errors: True
+
+  - name: Check that ansible reports failure
+    assert:
+      that:
+        - "not result.changed"
+        - "result.failed"
+        - "result.msg == 'Invalid role_attr_flags specified: INVALID'"
+
+  - name: Modify a single role attribute on a user
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: "present"
+      role_attr_flags: "LOGIN"
+      no_password_changes: '{{ no_password_changes }}'
+
+  - name: Check that ansible reports it modified the role
+    assert:
+      that:
+        - "result.changed"
+
+  - name: Check the role attributes
+    <<: *task_parameters
+    shell: echo "select 'super:'||rolsuper, 'createrole:'||rolcreaterole, 'create:'||rolcreatedb, 'inherit:'||rolinherit, 'login:'||rolcanlogin from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
+
+  - assert:
+      that:
+        - "result.stdout_lines[-1] == '(1 row)'"
+        - "'super:f' in result.stdout_lines[-2]"
+        - "'createrole:f' in result.stdout_lines[-2]"
+        - "'create:f' in result.stdout_lines[-2]"
+        - "'inherit:f' in result.stdout_lines[-2]"
+        - "'login:t' in result.stdout_lines[-2]"
+
+  - block:
+      - name: Check the role attribute BYPASSRLS
+        <<: *task_parameters
+        shell: echo "select 'bypassrls:'||rolbypassrls from pg_roles where rolname='{{ db_user1 }}';" | psql -d postgres
+
+      - assert:
+          that:
+            - "(postgres_version_resp.stdout | version_compare('9.5.0', '<')) or 'bypassrls:f' in result.stdout_lines[-2]"
+    when: bypassrls_supported
+
+  - name: Check that using same attribute a second time does nothing
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: "present"
+      role_attr_flags: "LOGIN"
+      no_password_changes: '{{ no_password_changes }}'
+    environment:
+        PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed
+
+  - name: Check there isn't any update reported
+    assert:
+      that:
+        - "not result.changed"
+
+  - name: Cleanup the user
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: 'absent'
+      no_password_changes: '{{ no_password_changes }}' # user deletion: no_password_changes is ignored
+
+  - name: Check that user was removed
+    <<: *task_parameters
+    shell: echo "select * from pg_user where usename='{{ db_user1 }}';" | psql -d postgres
+
+  - assert:
+      that:
+        - "result.stdout_lines[-1] == '(0 rows)'"
+
+  always:
+  - name: Cleanup the user
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      state: 'absent'

test/integration/targets/postgresql/tasks/test_password.yml

@@ -8,7 +8,7 @@
       name: "{{ db_user1 }}"
       login_user: "{{ pg_user }}"
 
-  block: # block is only used here in order to be able to define YAML anchors at the beginning in 'vars' section
+  block:
   - name: 'Check that PGOPTIONS environment variable is effective (1/2)'
     <<: *task_parameters
     postgresql_user:
@@ -63,6 +63,27 @@
         that:
           - "{{ not result|changed }}"
 
+  - name: 'Define an expiration time'
+    <<: *task_parameters
+    postgresql_user:
+      <<: *parameters
+      expires: '2025-01-01'
+    environment:
+      PGCLIENTENCODING: 'UTF8'
+
+  - <<: *changed
+
+  - name: 'Redefine the same expiration time'
+    <<: *task_parameters
+    postgresql_user:
+      expires: '2025-01-01'
+      <<: *parameters
+    environment:
+      PGCLIENTENCODING: 'UTF8'
+      PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed
+
+  - <<: *not_changed
+
   - block:
 
     - name: 'Using MD5-hashed password: check that password not changed when using cleartext password'
@@ -72,7 +93,7 @@
         password: '{{ db_password1 }}'
         encrypted: 'yes'
       environment:
-#        PGCLIENTENCODING: 'UTF8'
+        PGCLIENTENCODING: 'UTF8'
         PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed
 
     - <<: *not_changed
@@ -99,6 +120,18 @@
 
     - <<: *not_changed
 
+    - name: 'Redefine the same expiration time and password (encrypted)'
+      <<: *task_parameters
+      postgresql_user:
+        <<: *parameters
+        encrypted: 'yes'
+        password: "md5{{ (db_password1 ~ db_user1) | hash('md5')}}"
+        expires: '2025-01-01'
+      environment:
+        PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed
+
+    - <<: *not_changed
+
     - name: 'Using MD5-hashed password: check that password changed when using another cleartext password'
       <<: *task_parameters
       postgresql_user:
@@ -144,6 +177,19 @@
 
     - <<: *not_changed
 
+    - name: 'Redefine the same expiration time and password (not encrypted)'
+      <<: *task_parameters
+      postgresql_user:
+        <<: *parameters
+        password: "{{ db_password1 }}"
+        encrypted: 'no'
+        expires: '2025-01-01'
+      environment:
+        PGCLIENTENCODING: 'UTF8'
+        PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed
+
+    - <<: *not_changed
+
     - name: 'Using cleartext password: check that password changed when using another cleartext password'
       <<: *task_parameters
       postgresql_user:
@@ -184,3 +230,10 @@
         PGOPTIONS: '-c default_transaction_read_only=on' # ensure 'alter user' query isn't executed
 
   - <<: *not_changed
+
+  always:
+  - name: Remove user
+    <<: *task_parameters
+    postgresql_user:
+      state: 'absent'
+      <<: *parameters

test/sanity/pep8/legacy-files.txt

@@ -233,7 +233,6 @@ lib/ansible/modules/database/postgresql/postgresql_ext.py
 lib/ansible/modules/database/postgresql/postgresql_lang.py
 lib/ansible/modules/database/postgresql/postgresql_privs.py
 lib/ansible/modules/database/postgresql/postgresql_schema.py
-lib/ansible/modules/database/postgresql/postgresql_user.py
 lib/ansible/modules/database/vertica/vertica_configuration.py
 lib/ansible/modules/database/vertica/vertica_facts.py
 lib/ansible/modules/database/vertica/vertica_role.py