feature request: bulk option for pg_hba module

[betanummeric]

feature idea for module postgresql_pg_hba: bulk rule editing

Some of my pg_hba.conf have over 700 managed rules, which takes some time and memory to loop over (repeated parsing, editing and writing). My current usecase looks like this:

- name: manage pg_hba.conf
  community.postgresql.postgresql_pg_hba:
    create: yes
    dest: "{{ data_dir }}/pg_hba.conf"
    backup: yes
    backup_file: "{{ data_dir }}/pg_hba.conf.ansible_backup"
    keep_comments_at_rules: true
   # not used here: attributes, group, mode, owner, selevel, serole, setype, seuser, unsafe_writes

    contype: "{{ entry.host }}"
    method: "{{ entry.method }}"
    options: "{{ entry.options }}"
    address: "{{ entry.address }}"
    netmask: "{{ entry.netmask }}"
    databases: "{{ entry.databases }}"
    state: "{{ entry.state }}"
    users: "{{ entry.users }}"
    comment: "{{ entry.comment }}"
  loop: "{{ desired_state.pghba|default([]) }}"
  loop_control:
    loop_var: entry

I would like to optimize that by adding/removing multiple rules with a single call to the module, which could look like this:

- name: manage pg_hba.conf
  community.postgresql.postgresql_pg_hba:
    create: yes
    dest: "{{ data_dir }}/pg_hba.conf"
    backup: yes
    backup_file: "{{ data_dir }}/pg_hba.conf.ansible_backup"
    keep_comments_at_rules: true
   # not used here: attributes, group, mode, owner, selevel, serole, setype, seuser, unsafe_writes

    bulk: "{{ desired_state.pghba|default([]) }}"

The new bulk argument (or rules?) would take a list of objects, each specifying a rule using the existing arguments:

address
comment
contype
databases
method
netmask
options
state
users

If one object is missing one of these, the default is taken, if possible.

When the arguments which can be put into the bulk argument are set normally (example below), they could either

1. connflict with the bulk argument
2. be appended to the bulk argument
3. be treated as defaults for missing values in the bulk argument

- name: manage pg_hba.conf
  community.postgresql.postgresql_pg_hba:
    contype: host
    address: "2001:db8:1::/64"
    databases: db1
    users: user1
    state: absent

    bulk:
    - contype: host
      address: "2001:db8:2::/64"
      databases: db2
      users: user2
# option 1 would let the module fail

# option 2 would be equivalent to:
- name: manage pg_hba.conf
  community.postgresql.postgresql_pg_hba:
    bulk:
    - contype: host
      address: "2001:db8:1::/64"
      databases: db1
      users: user1
      state: absent
    - contype: host
      address: "2001:db8:2::/64"
      databases: db2
      users: user2
      # implicit "state: present"

# option 3 would be equivalent to:
- name: manage pg_hba.conf
  community.postgresql.postgresql_pg_hba:
    bulk:
    - contype: host
      address: "2001:db8:2::/64"
      databases: db2
      users: user2
      state: absent  # normal "state" argument used as default

[betanummeric]

Hi @jchancojr, @hunleyd, @tcraxs, @Andersson007, what do you think of this feature request? Is bulk a good name for the argument? How would you like it to work with the normal arguments (the three options)?
I can submit at PR.

[hunleyd]

I think I prefer rules over bulk.

If the user specifies args and the new bulk rules, I think we should basically do a combine on them behind the scenes such that values missing in the bulk rules are 'filled in' by specified args.

But, I defer to @Andersson007 @jchancojr @pgguru @keithf4 et al

[keithf4]

I like rules better as well. The combine would basically be the same result option 2, then?

[pgguru]

Agreed that rules is a nicer term. My initial response here is that this should be an either/or operation, and if you provide the rules param, you should reject if any of the other params are also provided. It also seems a little ambiguous in intent IMHO whether if you provide a batch of rules if we're effectively looping over each and performing the operation for each individual row vs "this is my pg_hba.conf rules, remove any other conflicting ones".

Since pg_hba.conf is significantly ordered, I presume the order of items in the rules would end up being the order of items in the file? What about if there are other non-user lines that need to occur first, or in an alternate order? (Disclaimer: I've not actually used this module, so there may already be known limitations/workarounds here, and this is just an optimization for repeated invocations, in which case this makes sense.)

[hunleyd]

The combine would basically be the same result option 2, then?

I was thinking like we do internally with _user and _merged .. it would overwrite anything specified in both, and fill-in-the-blanks for things not specified.

However, I'm now leaning towards what David said and thinking it might be best to either specify one or the other (individual rule, or bulk rules, but not both).

[betanummeric]

@pgguru The order of rules is not preserved because the module sorts them according to https://github.com/ansible-collections/community.postgresql/blob/2.1.5/plugins/modules/postgresql_pg_hba.py#L590-L625.
The documentation says:

The entries will be written out in a specific order. With this option you can control by which field they are ordered first, second and last. s=source, d=databases, u=users. This option is deprecated since 2.9 and will be removed in community.postgresql 3.0.0. Sortorder is now hardcoded to sdu.

I would implement the rules argument by iterating over the items and adding/removing the rules.
If you also want to remove rules which preexist but are not in the rules argument (so, overwrite the whole file) -- which I don't want -- we should add a switch for that. Maybe overwrite, boolean, off by default.

I would build the combine alias option 3 behavior as such:
The rule variable (e.g. method) is taken from the rules item, if present. When it is not there, the normal method argument value is taken. If that is missing too, the module default (md5) is used.

If we can't decide between conflicting or combining, we could let the users decide by adding a switch: rules_behavior, string, options conflict (default), combine

[jchancojr]

I like @betanummeric 's approach but the general idea of using rules with some sort of combine sounds good to me.

+1 for rules and combine

[hunleyd]

If we can't decide between conflicting or combining, we could let the users decide by adding a switch: rules_behavior, string, options conflict (default), combine

I'd be OK with this

If you also want to remove rules which preexist but are not in the rules argument (so, overwrite the whole file) -- which I don't want -- we should add a switch for that. Maybe overwrite, boolean, off by default.

or perhaps append (bool) defaulting to True ?

[betanummeric]

or perhaps append (bool) defaulting to True ?

I don't like append because with state: absent, a rule will get removed, not appended. Other names for the argument could be replace_rules (default: false) or remove_unknown (default: false).

Should this argument only apply when rules is set or also for a single, normally specified rule?

[hunleyd]

or perhaps append (bool) defaulting to True ?

I don't like append because with state: absent, a rule will get removed, not appended.

vgp!

Other names for the argument could be replace_rules (default: false) or remove_unknown (default: false).

Of those two, I prefer the former. But I prefer overwrite even more.

Should this argument only apply when rules is set or also for a single, normally specified rule?

My $0.02 it should apply always.

[Andersson007]

cc @sebasmannem