diff --git a/changelogs/fragments/1-community-postgresql_backports.yml b/changelogs/fragments/1-community-postgresql_backports.yml new file mode 100644 index 0000000000..26b2879c11 --- /dev/null +++ b/changelogs/fragments/1-community-postgresql_backports.yml @@ -0,0 +1,7 @@ +bugfixes: +- postgresql_query - fix decimal handling (https://github.com/ansible-collections/community.postgresql/issues/45). +- postgresql_query - fix datetime.timedelta type handling (https://github.com/ansible-collections/community.postgresql/issues/47). +- postgresql_ping - fix crash caused by wrong PgSQL version parsing (https://github.com/ansible-collections/community.postgresql/issues/40). +- postgresql_info - fix crash caused by wrong PgSQL version parsing (https://github.com/ansible-collections/community.postgresql/issues/40). +- postgresql_set - return a message instead of traceback when a passed parameter has not been found (https://github.com/ansible-collections/community.postgresql/issues/41). +- postgresql_set - fails in check_mode on non-numeric values containing ``B`` (https://github.com/ansible-collections/community.postgresql/issues/48). diff --git a/plugins/modules/database/postgresql/postgresql_info.py b/plugins/modules/database/postgresql/postgresql_info.py index aeec865170..cd7302d20f 100644 --- a/plugins/modules/database/postgresql/postgresql_info.py +++ b/plugins/modules/database/postgresql/postgresql_info.py @@ -926,7 +926,7 @@ class PgClusterInfo(object): raw = raw.split()[1].split('.') self.pg_info["version"] = dict( major=int(raw[0]), - minor=int(raw[1]), + minor=int(raw[1].rstrip(',')), ) def get_recovery_state(self): diff --git a/plugins/modules/database/postgresql/postgresql_ping.py b/plugins/modules/database/postgresql/postgresql_ping.py index 240cea579f..45c1c64339 100644 --- a/plugins/modules/database/postgresql/postgresql_ping.py +++ b/plugins/modules/database/postgresql/postgresql_ping.py @@ -116,7 +116,7 @@ class PgPing(object): raw = raw.split()[1].split('.') self.version = dict( major=int(raw[0]), - minor=int(raw[1]), + minor=int(raw[1].rstrip(',')), ) diff --git a/plugins/modules/database/postgresql/postgresql_query.py b/plugins/modules/database/postgresql/postgresql_query.py index e231fbd3c9..c70d2a674c 100644 --- a/plugins/modules/database/postgresql/postgresql_query.py +++ b/plugins/modules/database/postgresql/postgresql_query.py @@ -171,6 +171,27 @@ EXAMPLES = r''' search_path: - app1 - public + +# If you use a variable in positional_args / named_args that can +# be undefined and you wish to set it as NULL, the constructions like +# "{{ my_var if (my_var is defined) else none | default(none) }}" +# will not work as expected substituting an empty string instead of NULL. +# If possible, we suggest to use Ansible's DEFAULT_JINJA2_NATIVE configuration +# (https://docs.ansible.com/ansible/latest/reference_appendices/config.html#default-jinja2-native). +# Enabling it fixes this problem. If you cannot enable it, the following workaround +# can be used. +# You should precheck such a value and define it as NULL when undefined. +# For example: +- name: When undefined, set to NULL + set_fact: + my_var: NULL + when: my_var is undefined +# Then: +- name: Insert a value using positional arguments + community.postgresql.postgresql_query: + query: INSERT INTO test_table (col1) VALUES (%s) + positional_args: + - '{{ my_var }}' ''' RETURN = r''' @@ -222,6 +243,9 @@ rowcount: sample: 5 ''' +import datetime +import decimal + try: from psycopg2 import ProgrammingError as Psycopg2ProgrammingError from psycopg2.extras import DictCursor @@ -389,8 +413,20 @@ def main(): if cursor.rowcount > 0: rowcount += cursor.rowcount + query_result = [] try: - query_result = [dict(row) for row in cursor.fetchall()] + for row in cursor.fetchall(): + # Ansible engine does not support decimals. + # An explicit conversion is required on the module's side + row = dict(row) + for (key, val) in iteritems(row): + if isinstance(val, decimal.Decimal): + row[key] = float(val) + + elif isinstance(val, datetime.timedelta): + row[key] = str(val) + + query_result.append(row) except Psycopg2ProgrammingError as e: if to_native(e) == 'no results to fetch': diff --git a/plugins/modules/database/postgresql/postgresql_set.py b/plugins/modules/database/postgresql/postgresql_set.py index 737bded56e..da21f42a67 100644 --- a/plugins/modules/database/postgresql/postgresql_set.py +++ b/plugins/modules/database/postgresql/postgresql_set.py @@ -180,7 +180,7 @@ from ansible.module_utils._text import to_native PG_REQ_VER = 90400 # To allow to set value like 1mb instead of 1MB, etc: -POSSIBLE_SIZE_UNITS = ("mb", "gb", "tb") +LOWERCASE_SIZE_UNITS = ("mb", "gb", "tb") # =========================================== # PostgreSQL module specific support methods. @@ -199,6 +199,11 @@ def param_get(cursor, module, name): except Exception as e: module.fail_json(msg="Unable to get %s value due to : %s" % (name, to_native(e))) + if not info: + module.fail_json(msg="No such parameter: %s. " + "Please check its spelling or presence in your PostgreSQL version " + "(https://www.postgresql.org/docs/current/runtime-config.html)" % name) + raw_val = info[0][1] unit = info[0][2] context = info[0][3] @@ -233,32 +238,55 @@ def pretty_to_bytes(pretty_val): # if the value contains 'B', 'kB', 'MB', 'GB', 'TB'. # Otherwise it returns the passed argument. - val_in_bytes = None - - if 'kB' in pretty_val: - num_part = int(''.join(d for d in pretty_val if d.isdigit())) - val_in_bytes = num_part * 1024 - - elif 'MB' in pretty_val.upper(): - num_part = int(''.join(d for d in pretty_val if d.isdigit())) - val_in_bytes = num_part * 1024 * 1024 - - elif 'GB' in pretty_val.upper(): - num_part = int(''.join(d for d in pretty_val if d.isdigit())) - val_in_bytes = num_part * 1024 * 1024 * 1024 - - elif 'TB' in pretty_val.upper(): - num_part = int(''.join(d for d in pretty_val if d.isdigit())) - val_in_bytes = num_part * 1024 * 1024 * 1024 * 1024 - - elif 'B' in pretty_val.upper(): - num_part = int(''.join(d for d in pretty_val if d.isdigit())) - val_in_bytes = num_part - - else: + # It's sometimes possible to have an empty values + if not pretty_val: return pretty_val - return val_in_bytes + # If the first char is not a digit, it does not make sense + # to parse further, so just return a passed value + if not pretty_val[0].isdigit(): + return pretty_val + + # If the last char is not an alphabetical symbol, it means that + # it does not contain any suffixes, so no sense to parse further + if not pretty_val[-1].isalpha(): + return pretty_val + + # Extract digits + num_part = [] + for c in pretty_val: + # When we reach the first non-digit element, + # e.g. in 1024kB, stop iterating + if not c.isdigit(): + break + else: + num_part.append(c) + + num_part = ''.join(num_part) + + val_in_bytes = None + + if len(pretty_val) >= 2: + if 'kB' in pretty_val[-2:]: + val_in_bytes = num_part * 1024 + + elif 'MB' in pretty_val[-2:]: + val_in_bytes = num_part * 1024 * 1024 + + elif 'GB' in pretty_val[-2:]: + val_in_bytes = num_part * 1024 * 1024 * 1024 + + elif 'TB' in pretty_val[-2:]: + val_in_bytes = num_part * 1024 * 1024 * 1024 * 1024 + + # For cases like "1B" + if not val_in_bytes and 'B' in pretty_val[-1]: + val_in_bytes = num_part + + if val_in_bytes is not None: + return val_in_bytes + else: + return pretty_val def param_set(cursor, module, name, value, context): @@ -308,11 +336,14 @@ def main(): # Check input for potentially dangerous elements: check_input(module, name, value, session_role) - # Allow to pass values like 1mb instead of 1MB, etc: if value: - for unit in POSSIBLE_SIZE_UNITS: - if value[:-2].isdigit() and unit in value[-2:]: - value = value.upper() + # Convert a value like 1mb (Postgres does not support) to 1MB, etc: + if len(value) > 2 and value[:-2].isdigit() and value[-2:] in LOWERCASE_SIZE_UNITS: + value = value.upper() + + # Convert a value like 1b (Postgres does not support) to 1B: + elif len(value) > 1 and ('b' in value[-1] and value[:-1].isdigit()): + value = value.upper() if value is not None and reset: module.fail_json(msg="%s: value and reset params are mutually exclusive" % name) diff --git a/plugins/modules/database/postgresql/postgresql_user.py b/plugins/modules/database/postgresql/postgresql_user.py index 79c987a774..cdede5867c 100644 --- a/plugins/modules/database/postgresql/postgresql_user.py +++ b/plugins/modules/database/postgresql/postgresql_user.py @@ -107,6 +107,7 @@ options: no_password_changes: description: - If C(yes), does not inspect the database for password changes. + If the user already exists, skips all password related checks. Useful when C(pg_authid) is not accessible (such as in AWS RDS). Otherwise, makes password changes as necessary. default: no @@ -156,6 +157,10 @@ notes: On the previous versions the whole hashed string is used as a password. - 'Working with SCRAM-SHA-256-hashed passwords, be sure you use the I(environment:) variable C(PGOPTIONS: "-c password_encryption=scram-sha-256") (see the provided example).' +- On some systems (such as AWS RDS), C(pg_authid) is not accessible, thus, the module cannot compare + the current and desired C(password). In this case, the module assumes that the passwords are + different and changes it reporting that the state has been changed. + To skip all password related checks for existing users, use I(no_password_changes=yes). - Supports ``check_mode``. seealso: - module: community.general.postgresql_privs diff --git a/tests/integration/targets/postgresql_query/tasks/postgresql_query_initial.yml b/tests/integration/targets/postgresql_query/tasks/postgresql_query_initial.yml index 6f71480c65..339f588bad 100644 --- a/tests/integration/targets/postgresql_query/tasks/postgresql_query_initial.yml +++ b/tests/integration/targets/postgresql_query/tasks/postgresql_query_initial.yml @@ -532,3 +532,68 @@ - assert: that: - result is failed + +############################################################################# +# Issue https://github.com/ansible-collections/community.postgresql/issues/45 +- name: Create table containing a decimal value + become_user: '{{ pg_user }}' + become: true + postgresql_query: + login_user: '{{ pg_user }}' + db: postgres + query: CREATE TABLE blabla (id int, num decimal) + +- name: Insert data + become_user: '{{ pg_user }}' + become: true + postgresql_query: + login_user: '{{ pg_user }}' + db: postgres + query: INSERT INTO blabla (id, num) VALUES (1, 1::decimal) + +- name: Get data + become_user: '{{ pg_user }}' + become: true + postgresql_query: + login_user: '{{ pg_user }}' + db: postgres + query: SELECT * FROM blabla + register: result + +- assert: + that: + - result.rowcount == 1 + +############################################################################# +# Issue https://github.com/ansible-collections/community.postgresql/issues/47 +- name: Get datetime.timedelta value + become_user: '{{ pg_user }}' + become: true + postgresql_query: + login_user: '{{ pg_user }}' + db: postgres + query: "SELECT EXTRACT(epoch from make_interval(secs => 3))" + register: result + when: postgres_version_resp.stdout is version('10', '>=') + +- assert: + that: + - result.rowcount == 1 + - result.query_result[0]["date_part"] == 3 + when: postgres_version_resp.stdout is version('10', '>=') + +- name: Get interval value + become_user: '{{ pg_user }}' + become: true + postgresql_query: + login_user: '{{ pg_user }}' + db: postgres + query: "SELECT make_interval(secs => 3)" + register: result + when: postgres_version_resp.stdout is version('10', '>=') + +- assert: + that: + - result.rowcount == 1 + - result.query_result[0]["make_interval"] == "0:00:03" + when: postgres_version_resp.stdout is version('10', '>=') diff --git a/tests/integration/targets/postgresql_set/tasks/main.yml b/tests/integration/targets/postgresql_set/tasks/main.yml index 9750fff769..3f16eb0d69 100644 --- a/tests/integration/targets/postgresql_set/tasks/main.yml +++ b/tests/integration/targets/postgresql_set/tasks/main.yml @@ -6,3 +6,6 @@ # Initial CI tests of postgresql_initial module - include_tasks: postgresql_set_initial.yml when: postgres_version_resp.stdout is version('9.6', '>=') + +- include_tasks: options_coverage.yml + when: postgres_version_resp.stdout is version('9.6', '>=') diff --git a/tests/integration/targets/postgresql_set/tasks/options_coverage.yml b/tests/integration/targets/postgresql_set/tasks/options_coverage.yml new file mode 100644 index 0000000000..acb940e587 --- /dev/null +++ b/tests/integration/targets/postgresql_set/tasks/options_coverage.yml @@ -0,0 +1,50 @@ +# Test code for the postgresql_set module +# Copyright: (c) 2021, Andrew Klychkov (@Andersson007) +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +- vars: + task_parameters: &task_parameters + become_user: '{{ pg_user }}' + become: yes + pg_parameters: &pg_parameters + login_user: '{{ pg_user }}' + login_db: postgres + + block: + - name: Define a test setting map + set_fact: + setting_map: + allow_system_table_mods: on + archive_command: /bin/true + archive_timeout: 10min + autovacuum_work_mem: '-1' + backend_flush_after: 0 + autovacuum_vacuum_scale_factor: 0.5 + client_encoding: UTF-8 + bgwriter_delay: 400 + maintenance_work_mem: 32mb + effective_cache_size: 1024kB + shared_buffers: 1GB + stats_temp_directory: pg_stat_tmp + wal_level: replica + log_statement: mod + track_functions: none + + # Check mode: + - name: Set settings in check mode + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: '{{ item.key }}' + value: '{{ item.value }}' + check_mode: yes + with_dict: '{{ setting_map }}' + + # Actual mode: + - name: Set settings in actual mode + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: '{{ item.key }}' + value: '{{ item.value }}' + with_dict: '{{ setting_map }}' diff --git a/tests/integration/targets/postgresql_set/tasks/postgresql_set_initial.yml b/tests/integration/targets/postgresql_set/tasks/postgresql_set_initial.yml index af0502af09..125e1daba0 100644 --- a/tests/integration/targets/postgresql_set/tasks/postgresql_set_initial.yml +++ b/tests/integration/targets/postgresql_set/tasks/postgresql_set_initial.yml @@ -373,3 +373,70 @@ - assert: that: - result is not changed + + - name: Pass non-existent parameter + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: Timezone + value: utc + register: result + ignore_errors: yes + + - assert: + that: + - result is failed + - result.msg is search('No such parameter') + + ####################################################################### + # https://github.com/ansible-collections/community.postgresql/issues/48 + - name: Pass a parameter containing b in check_mode + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: archive_command + value: '/usr/bin/touch %f' + register: result + check_mode: yes + + - assert: + that: + - result is changed + + - name: Pass a parameter containing b + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: archive_command + value: '/usr/bin/touch %f' + register: result + + - assert: + that: + - result is changed + + - name: Pass another parameter containing B in check_mode + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: track_activity_query_size + value: '4096B' + register: result + check_mode: yes + + - assert: + that: + - result is changed + + - name: Pass another parameter containing b in check_mode + <<: *task_parameters + postgresql_set: + <<: *pg_parameters + name: track_activity_query_size + value: '2048b' + register: result + check_mode: yes + + - assert: + that: + - result is changed