The keep_comments_at_rules option of postgresql_pg_hba breaks idempotency

[toydarian]

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]

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]

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...

[toydarian]

@betanummeric What I propose is to change the behavior to always be keep_comments_at_rules=true. I see very little reason for it to be false.

The required changes are already implemented in #778 (that one needs more work, but you can take a look).

Using the bulk-rules feature in combination with turning off sorting will give you full flexibility with comments that are not associated with a specific rule at the expense of having to sort your rules manually.
Otherwise you would be limited to one global comment which you can make multi-line with / in front of the new line character.

I consider broken idempotency a bug and I want to fix that, even if it means implementing a new feature.

[Andersson007]

i have no opinion on this proposal as I don't use the module.
An important thing is the backwards compatibility.

• Can it break current playbooks or worse postgres configuration?
• If no, cool, I think we could proceed as there's no opposition so far
• If yes (even if the probability is low):
  • The option (or functionality) should be deprecated right now with a note in the changelog about it / or I would suggest changing the default from false to true and to notify folks about it maybe?
  • A warning should be added if possible when users use the deprecated stuff (or use the default value)
  • All other preparatory work should be done now
  • An issue for the breaking changes should be created and added to milestone for version 4.0.0 (we can do it in May)