The `keep_comments_at_rules` option of `postgresql_pg_hba` breaks idempotency

toydarian commented 1 day ago

SUMMARY

if keep_comments_at_rules is false (which is the default) this breaks idempotency

ISSUE TYPE

Bug Report

COMPONENT NAME

postgresql_pg_hba

ANSIBLE VERSION

% ansible --version
ansible [core 2.17.0]
  config file = None
  configured module search path = ['/home/tommy/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/tommy/venvs/ansible/lib/python3.11/site-packages/ansible
  ansible collection location = /home/tommy/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/tommy/venvs/ansible/bin/ansible
  python version = 3.11.9 (main, Jul 11 2024, 14:01:32) [GCC 9.4.0] (/home/tommy/venvs/ansible/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True

COLLECTION VERSION

All versions of the collections since 1.5.0

STEPS TO REPRODUCE

Run this playbook twice:

- hosts: all
  tasks:

    - name: Create a rule with the comment 'comment1'
      postgresql_pg_hba:
        contype: host
        dest: /tmp/pg_hba2.conf
        create: true
        method: md5
        address: "2001:db8::1/128"
        state: present
        comment: "comment1"
        keep_comments_at_rules: false

    - shell: cat /tmp/pg_hba2.conf
      register: out1
    - debug:
        var: out1

    - name: Create a rule with the comment 'comment2'
      postgresql_pg_hba:
        contype: host
        dest: /tmp/pg_hba2.conf
        method: md5
        address: "2001:db8::2/128"
        state: present
        comment: "comment2"
        keep_comments_at_rules: false

    - shell: cat /tmp/pg_hba2.conf
      register: out1
    - debug:
        var: out1

% ansible-playbook -i inv.yml pg-hba-test.yml

PLAY [all] ***********************************************************************************************************************************************************************************

TASK [Gathering Facts] ***********************************************************************************************************************************************************************
ok: [local]

TASK [Create a rule with the comment 'comment1'] *********************************************************************************************************************************************
changed: [local]

TASK [shell] *********************************************************************************************************************************************************************************
changed: [local]

TASK [debug] *********************************************************************************************************************************************************************************
ok: [local] => {
    "out1": {
        "changed": true,
...
        "stdout": "\nhost\tall\tall\t2001:db8::1/128\tmd5\t#comment1",
        "stdout_lines": [
            "",
            "host\tall\tall\t2001:db8::1/128\tmd5\t#comment1"
        ]
    }
}

TASK [Create a rule with the comment 'comment2'] *********************************************************************************************************************************************
changed: [local]

TASK [shell] *********************************************************************************************************************************************************************************
changed: [local]

TASK [debug] *********************************************************************************************************************************************************************************
ok: [local] => {
    "out1": {
        "changed": true,
...
        "stdout": "#comment1\nhost\tall\tall\t2001:db8::1/128\tmd5\nhost\tall\tall\t2001:db8::2/128\tmd5\t#comment2",
        "stdout_lines": [
            "#comment1",
            "host\tall\tall\t2001:db8::1/128\tmd5",
            "host\tall\tall\t2001:db8::2/128\tmd5\t#comment2"
        ]
    }
}

PLAY RECAP ***********************************************************************************************************************************************************************************
local                      : ok=7    changed=4    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

tommy@passau ~/test % ansible-playbook -i inv.yml pg-hba-test.yml

PLAY [all] ***********************************************************************************************************************************************************************************

TASK [Gathering Facts] ***********************************************************************************************************************************************************************
ok: [local]

TASK [Create a rule with the comment 'comment1'] *********************************************************************************************************************************************
changed: [local]

TASK [shell] *********************************************************************************************************************************************************************************
changed: [local]

TASK [debug] *********************************************************************************************************************************************************************************
ok: [local] => {
    "out1": {
        "changed": true,
...
        "stdout": "#comment1\n#comment2\nhost\tall\tall\t2001:db8::1/128\tmd5\t#comment1\nhost\tall\tall\t2001:db8::2/128\tmd5",
        "stdout_lines": [
            "#comment1",
            "#comment2",
            "host\tall\tall\t2001:db8::1/128\tmd5\t#comment1",
            "host\tall\tall\t2001:db8::2/128\tmd5"
        ]
    }
}

TASK [Create a rule with the comment 'comment2'] *********************************************************************************************************************************************
changed: [local]

TASK [shell] *********************************************************************************************************************************************************************************
changed: [local]

TASK [debug] *********************************************************************************************************************************************************************************
ok: [local] => {
    "out1": {
        "changed": true,
...
        "stdout": "#comment1\n#comment2\n#comment1\nhost\tall\tall\t2001:db8::1/128\tmd5\nhost\tall\tall\t2001:db8::2/128\tmd5\t#comment2",
        "stdout_lines": [
            "#comment1",
            "#comment2",
            "#comment1",
            "host\tall\tall\t2001:db8::1/128\tmd5",
            "host\tall\tall\t2001:db8::2/128\tmd5\t#comment2"
        ]
    }
}

PLAY RECAP ***********************************************************************************************************************************************************************************
local                      : ok=7    changed=4    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

EXPECTED RESULTS

As shown above the same comments are added and moved over and over again

ACTUAL RESULTS

The comments don't get added several times

SUGGESTED FIX

Completely remove the keep_comments_at_rules option and add a way to add a comment to the top of the file.

hunleyd commented 1 day ago

i concur w/ the proposed removal of the option. i'm not sure we need to support adding 'out-of-line' comments. seems like if the user wants that, they could use the template module to build the file. my $0.02

betanummeric commented 4 hours ago

I introduced the keep_comments_at_rules and comment in #135. comment was only intended to be used together with keep_comments_at_rules: true. I'm using the feature to write justifications behind each rules and preserve those comments. I want to preserve this feature.

Using a comment with keep_comments_at_rules: false is indeed not idempotent. I see multiple ways to handle this:

log a warning
throw an error
remove keep_comments_at_rules and always run with keep_comments_at_rules: true behavior (avoiding this change was the reason for keep_comments_at_rules)

Supporting management of comments which don't belong to a specific rule (for example gathered at the top of the pg_hba.conf) would be nice, but I think that's a new feature and therefore low priority. @hunleyd The postgresql_pg_hba module offers advanced features which a template doesn't have. Using postgresql_pg_hba for the rules, together with a lineinfile for standalone comments is probably a better solution. Although this may break if there are multi-line rules in the file...

ansible-collections / community.postgresql