Better handling of extension versions (#163)

* Initial changes to postgresql extension handling * Properly handle latest version flag. Formatting fixes. * Fix variable usage. Also make param usage consistent. Update docs. * Update plugins/modules/postgresql_ext.py Co-authored-by: Douglas J Hunley <doug.hunley@gmail.com> * Update plugins/modules/postgresql_ext.py Co-authored-by: Douglas J Hunley <doug.hunley@gmail.com> * Fix CI test failures * Wrap long doc line * Fix invalid yaml in examples * Fix handling given version on extension creation. Use safe sql string generation with identifiers. * Fix CI test failures * Revert safe sql string building. Not supported in CentOS7/U20 * Fix CI Test on Ubuntu to use new extension update syntax for latest version * Fix CI test for U20 when updating to latest version * Fix CI test for non-existent extension. Update python doc. * Remove unused codepath * Remove extra comment marker * Add changlog fragment * Apply suggestions from code review Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> * Make available_extensions a set. Remove SELECT from executed queries. Update doc. Co-authored-by: Douglas J Hunley <doug.hunley@gmail.com> Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
ansible-collections · Nov 24, 2021 · 8eaf1af · 8eaf1af
1 parent 44dc453
commit 8eaf1af
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 88 deletions.
diff --git a/changelogs/fragments/163-better_handling_of_postgresql_extensions.yml b/changelogs/fragments/163-better_handling_of_postgresql_extensions.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- postgresql_ext - Handle postgresql extension updates through path validation instead of version comparison (https://github.com/ansible-collections/community.postgresql/issues/129).
diff --git a/plugins/modules/postgresql_ext.py b/plugins/modules/postgresql_ext.py
@@ -71,10 +71,13 @@
   version:
     description:
       - Extension version to add or update to. Has effect with I(state=present) only.
-      - If not specified, the latest extension version will be created.
-      - It can't downgrade an extension version.
-        When version downgrade is needed, remove the extension and create new one with appropriate version.
-      - Set I(version=latest) to update the extension to the latest available version.
+      - If not specified and extension is not installed in the database,
+        the latest version available will be created.
+      - If extension is already installed, will update to the given version if a valid update
+        path exists.
+      - Downgrading is only supported if the extension provides a downgrade path otherwise
+        the extension must be removed and a lower version of the extension must be made available.
+      - Set I(version=latest) to always update the extension to the latest available version.
     type: str
   trust_input:
     description:
@@ -114,6 +117,7 @@
 - Thomas O'Donnell (@andytom)
 - Sandro Santilli (@strk)
 - Andrew Klychkov (@Andersson007)
+- Keith Fiske (@keithf4)
 extends_documentation_fragment:
 - community.postgresql.postgres
 
@@ -147,13 +151,13 @@
     cascade: yes
     state: absent
 
-- name: Create extension foo of version 1.2 or update it if it's already created
+- name: Create extension foo of version 1.2 or update it to that version if it's already created and a valid update path exists
   community.postgresql.postgresql_ext:
     db: acme
     name: foo
     version: 1.2
 
-- name: Assuming extension foo is created, update it to the latest version
+- name: Create the latest available version of extension foo. If already installed, update it to the latest version
   community.postgresql.postgresql_ext:
     db: acme
     name: foo
@@ -171,8 +175,6 @@
 
 import traceback
 
-from distutils.version import LooseVersion
-
 try:
     from psycopg2.extras import DictCursor
 except ImportError:
@@ -198,19 +200,26 @@
 # PostgreSQL module specific support methods.
 #
 
-def ext_exists(cursor, ext):
-    query = "SELECT * FROM pg_extension WHERE extname=%(ext)s"
-    cursor.execute(query, {'ext': ext})
-    return cursor.rowcount == 1
 
+def ext_delete(cursor, ext, current_version, cascade):
+    """Remove the extension from the database.
+
+    Return True if success.
 
-def ext_delete(cursor, ext, cascade):
-    if ext_exists(cursor, ext):
+    Args:
+      cursor (cursor) -- cursor object of psycopg2 library
+      ext (str) -- extension name
+      current_version (str) -- installed version of the extension.
+        Value obtained from ext_get_versions and used to
+        determine if the extension was installed.
+      cascade (boolean) -- Pass the CASCADE flag to the DROP commmand
+    """
+    if current_version:
         query = "DROP EXTENSION \"%s\"" % ext
         if cascade:
             query += " CASCADE"
         cursor.execute(query)
-        executed_queries.append(query)
+        executed_queries.append(cursor.mogrify(query))
         return True
     else:
         return False
@@ -240,12 +249,23 @@ def ext_update_version(cursor, ext, version):
 
 
 def ext_create(cursor, ext, schema, cascade, version):
+    """
+    Create the extension objects inside the database.
+
+    Return True if success.
+
+    Args:
+      cursor (cursor) -- cursor object of psycopg2 library
+      ext (str) -- extension name
+      schema (str) -- target schema for extension objects
+      version (str) -- extension version
+    """
     query = "CREATE EXTENSION \"%s\"" % ext
     params = {}
 
     if schema:
         query += " WITH SCHEMA \"%s\"" % schema
-    if version:
+    if version != 'latest':
         query += " VERSION %(ver)s"
         params['ver'] = version
     if cascade:
@@ -258,7 +278,9 @@ def ext_create(cursor, ext, schema, cascade, version):
 
 def ext_get_versions(cursor, ext):
     """
-    Get the current created extension version and available versions.
+    Get the currently created extension version if it is installed
+    in the database and versions that are available if it is
+    installed on the system.
 
     Return tuple (current_version, [list of available versions]).
 
@@ -272,60 +294,70 @@ def ext_get_versions(cursor, ext):
       ext (str) -- extension name
     """
 
+    current_version = None
+    params = {}
+    params['ext'] = ext
+
     # 1. Get the current extension version:
     query = ("SELECT extversion FROM pg_catalog.pg_extension "
              "WHERE extname = %(ext)s")
 
-    current_version = None
-    cursor.execute(query, {'ext': ext})
+    cursor.execute(query, params)
+
     res = cursor.fetchone()
     if res:
         current_version = res[0]
 
     # 2. Get available versions:
     query = ("SELECT version FROM pg_available_extension_versions "
              "WHERE name = %(ext)s")
-    cursor.execute(query, {'ext': ext})
-    res = cursor.fetchall()
 
-    available_versions = parse_ext_versions(current_version, res)
+    cursor.execute(query, params)
+
+    available_versions = set(r[0] for r in cursor.fetchall())
 
     if current_version is None:
         current_version = False
 
     return (current_version, available_versions)
 
 
-def parse_ext_versions(current_version, ext_ver_list):
-    """Parse ext versions.
+def ext_valid_update_path(cursor, ext, current_version, version):
+    """
+    Check to see if the installed extension version has a valid update
+    path to the given version. A version of 'latest' is always a valid path.
+
+    Return True if a valid path exists. Otherwise return False.
 
     Args:
-      current_version (str) -- version to compare elements of ext_ver_list with
-      ext_ver_list (list) -- list containing dicts with versions
+      cursor (cursor) -- cursor object of psycopg2 library
+      ext (str) -- extension name
+      current_version (str) -- installed version of the extension.
+      version (str) -- target extension version to update to.
+        A value of 'latest' is always a valid path and will result
+        in the extension update command always being run.
+    """
 
-    Return a sorted list with versions that are higher than current_version.
+    valid_path = False
+    params = {}
+    if version != 'latest':
+        query = ("SELECT path FROM pg_extension_update_paths(%(ext)s) "
+                 "WHERE source = %(cv)s "
+                 "AND target = %(ver)s")
 
-    Note: Incomparable versions (e.g., postgis version "unpackaged") are skipped.
-    """
-    available_versions = []
+        params['ext'] = ext
+        params['cv'] = current_version
+        params['ver'] = version
 
-    for line in ext_ver_list:
-        if line['version'] == 'unpackaged':
-            continue
+        cursor.execute(query, params)
+        res = cursor.fetchone()
+        if res is not None:
+            valid_path = True
+    else:
+        valid_path = True
 
-        try:
-            if current_version is None:
-                if LooseVersion(line['version']) >= LooseVersion('0'):
-                    available_versions.append(line['version'])
-            else:
-                if LooseVersion(line['version']) > LooseVersion(current_version):
-                    available_versions.append(line['version'])
-        except Exception:
-            # When a version cannot be compared, skip it
-            # (there's a note in the documentation)
-            continue
+    return (valid_path)
 
-    return sorted(available_versions, key=LooseVersion)
 
 # ===========================================
 # Module execution.
@@ -374,71 +406,67 @@ def main():
         curr_version, available_versions = ext_get_versions(cursor, ext)
 
         if state == "present":
-            if version == 'latest':
-                if available_versions:
-                    version = available_versions[-1]
-                else:
-                    version = ''
 
+            # If version passed
             if version:
-                # If the specific version is passed and it is not available for update:
-                if version not in available_versions:
-                    if not curr_version:
-                        module.fail_json(msg="Passed version '%s' is not available" % version)
-
-                    elif LooseVersion(curr_version) == LooseVersion(version):
+                # If extension is installed, update to passed version if a valid path exists
+                if curr_version:
+                    # Given version already installed
+                    if curr_version == version:
                         changed = False
-
+                    # Attempt to update to given version or latest version defined in extension control file
+                    # ALTER EXTENSION is actually run if valid, so 'changed' will be true even if nothing updated
+                    else:
+                        valid_update_path = ext_valid_update_path(cursor, ext, curr_version, version)
+                        if valid_update_path:
+                            if module.check_mode:
+                                changed = True
+                            else:
+                                changed = ext_update_version(cursor, ext, version)
+                        else:
+                            module.fail_json(msg="Passed version '%s' has no valid update path from "
+                                                 "the currently installed version '%s' or "
+                                                 "the passed version is not available" % (version, curr_version))
+                else:
+                    # If not requesting latest version and passed version not available
+                    if version != 'latest' and version not in available_versions:
+                        module.fail_json(msg="Passed version '%s' is not available" % version)
+                    # Else install the passed version when available
                     else:
-                        module.fail_json(msg="Passed version '%s' is lower than "
-                                             "the current created version '%s' or "
-                                             "the passed version is not available" % (version, curr_version))
-
-                # If the specific version is passed and it is higher that the current version:
-                if curr_version:
-                    if LooseVersion(curr_version) < LooseVersion(version):
                         if module.check_mode:
                             changed = True
                         else:
-                            changed = ext_update_version(cursor, ext, version)
-
-                    # If the specific version is passed and it is created now:
-                    if curr_version == version:
-                        changed = False
-
-                # If the ext doesn't exist and installed:
-                elif not curr_version and available_versions:
-                    if module.check_mode:
-                        changed = True
-                    else:
-                        changed = ext_create(cursor, ext, schema, cascade, version)
+                            changed = ext_create(cursor, ext, schema, cascade, version)
 
             # If version is not passed:
             else:
-                if not curr_version:
-                    # If the ext doesn't exist and it's installed:
+                # Extension exists, no request to update so no change
+                if curr_version:
+                    changed = False
+                else:
+                    # If the ext doesn't exist and is available:
                     if available_versions:
                         if module.check_mode:
                             changed = True
                         else:
-                            changed = ext_create(cursor, ext, schema, cascade, version)
+                            changed = ext_create(cursor, ext, schema, cascade, 'latest')
 
-                    # If the ext doesn't exist and not installed:
+                    # If the ext doesn't exist and is not available:
                     else:
-                        module.fail_json(msg="Extension %s is not installed" % ext)
+                        module.fail_json(msg="Extension %s is not available" % ext)
 
         elif state == "absent":
             if curr_version:
                 if module.check_mode:
                     changed = True
                 else:
-                    changed = ext_delete(cursor, ext, cascade)
+                    changed = ext_delete(cursor, ext, curr_version, cascade)
             else:
                 changed = False
 
     except Exception as e:
         db_connection.close()
-        module.fail_json(msg="Database query failed: %s" % to_native(e), exception=traceback.format_exc())
+        module.fail_json(msg="Management of PostgreSQL extension failed: %s" % to_native(e), exception=traceback.format_exc())
 
     db_connection.close()
     module.exit_json(changed=changed, db=module.params["db"], ext=ext, queries=executed_queries)

diff --git a/tests/integration/targets/postgresql_ext/tasks/postgresql_ext_version_opt.yml b/tests/integration/targets/postgresql_ext/tasks/postgresql_ext_version_opt.yml
@@ -199,7 +199,7 @@
   - assert:
       that:
       - result is changed
-      - result.queries == ["ALTER EXTENSION \"{{ test_ext }}\" UPDATE TO '3.0'"]
+      - result.queries == ["ALTER EXTENSION \"{{ test_ext }}\" UPDATE"]
 
   - name: postgresql_ext_version - check
     <<: *task_parameters
@@ -211,7 +211,7 @@
       that:
       - result.rowcount == 1
 
-  - name: postgresql_ext_version - try to update the extension to the latest version again
+  - name: postgresql_ext_version - try to update the extension to the latest version again which always runs an update.
     <<: *task_parameters
     postgresql_ext:
       <<: *pg_parameters
@@ -222,7 +222,17 @@
 
   - assert:
       that:
-      - result is not changed
+      - result is changed
+
+  - name: postgresql_ext_version - check that version number did not change even though update ran
+    <<: *task_parameters
+    postgresql_query:
+      <<: *pg_parameters
+      query: "SELECT 1 FROM pg_extension WHERE extname = '{{ test_ext }}' AND extversion = '3.0'"
+
+  - assert:
+      that:
+      - result.rowcount == 1
 
   - name: postgresql_ext_version - try to downgrade the extension version, must fail
     <<: *task_parameters
@@ -317,7 +327,7 @@
       that:
       - result.rowcount == 1
 
-  - name: postgresql_ext_version - try to install non-existent version
+  - name: postgresql_ext_version - try to install non-existent extension
     <<: *task_parameters
     postgresql_ext:
       <<: *pg_parameters
@@ -328,7 +338,7 @@
   - assert:
       that:
       - result.failed == true
-      - result.msg == "Extension non_existent is not installed"
+      - result.msg == "Extension non_existent is not available"
 
   ######################################################################
   # https://github.com/ansible-collections/community.general/issues/1095
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		bugfixes:
		- postgresql_ext - Handle postgresql extension updates through path validation instead of version comparison (https://github.com/ansible-collections/community.postgresql/issues/129).