diff --git a/changelogs/fragments/57147_Fix_postgresql_pg_hba_TypeError_pg_hba.conf_wiped.yml b/changelogs/fragments/57147_Fix_postgresql_pg_hba_TypeError_pg_hba.conf_wiped.yml new file mode 100644 index 0000000000..dff34002b0 --- /dev/null +++ b/changelogs/fragments/57147_Fix_postgresql_pg_hba_TypeError_pg_hba.conf_wiped.yml @@ -0,0 +1,2 @@ +bugfixes: +- "postgresql_pg_hba - Fix TypeError after which pg_hba.conf is wiped (https://github.com/ansible/ansible/issues/56430)" diff --git a/lib/ansible/modules/database/postgresql/postgresql_pg_hba.py b/lib/ansible/modules/database/postgresql/postgresql_pg_hba.py index a9f5ab66ae..acff04e966 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_pg_hba.py +++ b/lib/ansible/modules/database/postgresql/postgresql_pg_hba.py @@ -82,8 +82,10 @@ options: order: description: - 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. + 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 2.11. + Sortorder is now hardcoded to sdu. default: sdu choices: [ sdu, sud, dsu, dus, usd, uds ] state: @@ -301,6 +303,7 @@ class PgHba(object): if not self.changed(): return False + contents = self.render() if self.pg_hba_file: if not (os.path.isfile(self.pg_hba_file) or self.create): raise PgHbaError("pg_hba file '{0}' doesn't exist. " @@ -316,7 +319,7 @@ class PgHba(object): filed, __path = tempfile.mkstemp(prefix='pg_hba') fileh = os.fdopen(filed, 'w') - fileh.write(self.render()) + fileh.write(contents) self.unchanged() fileh.close() return True @@ -364,11 +367,7 @@ class PgHba(object): ''' This method returns all the rules of the PgHba object ''' - rules = sorted(self.rules.values(), - key=lambda rule: rule.weight(self.order, - len(self.users) + 1, - len(self.databases) + 1), - reverse=True) + rules = sorted(self.rules.values()) for rule in rules: ret = {} for key, value in rule.items(): @@ -544,8 +543,12 @@ class PgHbaRule(dict): except ValueError: return self['src'] - def weight(self, order, numusers, numdbs): - ''' + def __lt__(self, other): + """This function helps sorted to decide how to sort. + + It just checks itself against the other and decides on some key values + if it should be sorted higher or lower in the list. + The way it works: For networks, every 1 in 'netmask in binary' makes the subnet more specific. Therefore I chose to use prefix as the weight. So a single IP (/32) should have twice the weight of a /16 network. @@ -554,69 +557,100 @@ class PgHbaRule(dict): - for ipv4, we use a weight scale of 0 (all possible ipv4 addresses) to 128 (single ip) Therefore for ipv4, we use prefixlen (0-32) * 4 for weight, which corresponds to ipv6 (0-128). - ''' - if order not in PG_HBA_ORDERS: - raise PgHbaRuleError('{0} is not a valid order'.format(order)) + """ + myweight = self.source_weight() + hisweight = other.source_weight() + if myweight != hisweight: + return myweight > hisweight + myweight = self.db_weight() + hisweight = other.db_weight() + if myweight != hisweight: + return myweight < hisweight + + myweight = self.user_weight() + hisweight = other.user_weight() + if myweight != hisweight: + return myweight < hisweight + try: + return self['src'] < other['src'] + except TypeError: + return self.source_type_weight() < other.source_type_weight() + errormessage = 'We have two rules ({1}, {2})'.format(self, other) + errormessage += ' with exact same weight. Please file a bug.' + raise PgHbaValueError(errormessage) + + def source_weight(self): + """Report the weight of this source net. + + Basically this is the netmask, where IPv4 is normalized to IPv6 + (IPv4/32 has the same weight as IPv6/128). + """ if self['type'] == 'local': - sourceobj = '' - # local is always 'this server' and therefore considered /32 - srcweight = 130 # (Sort local on top of all) - else: - sourceobj = self.source() - if isinstance(sourceobj, ipaddress.IPv4Network): - srcweight = sourceobj.prefixlen * 4 - elif isinstance(sourceobj, ipaddress.IPv6Network): - srcweight = sourceobj.prefixlen - elif isinstance(sourceobj, str): - # You can also write all to match any IP address, - # samehost to match any of the server's own IP addresses, - # or samenet to match any address in any subnet that the server is connected to. - if sourceobj == 'all': - # (all is considered the full range of all ips, which has a weight of 0) - srcweight = 0 - elif sourceobj == 'samehost': - # (sort samehost second after local) - srcweight = 129 - elif sourceobj == 'samenet': - # Might write some fancy code to determine all prefix's - # from all interfaces and find a sane value for this one. - # For now, let's assume IPv4/24 or IPv6/96 (both have weight 96). - srcweight = 96 - elif sourceobj[0] == '.': - # suffix matching (domain name), let's asume a very large scale - # and therefore a very low weight IPv4/16 or IPv6/64 (both have weight 64). - srcweight = 64 - else: - # hostname, let's asume only one host matches, which is - # IPv4/32 or IPv6/128 (both have weight 128) - srcweight = 128 + return 130 + sourceobj = self.source() + if isinstance(sourceobj, ipaddress.IPv4Network): + return sourceobj.prefixlen * 4 + if isinstance(sourceobj, ipaddress.IPv6Network): + return sourceobj.prefixlen + if isinstance(sourceobj, str): + # You can also write all to match any IP address, + # samehost to match any of the server's own IP addresses, + # or samenet to match any address in any subnet that the server is connected to. + if sourceobj == 'all': + # (all is considered the full range of all ips, which has a weight of 0) + return 0 + if sourceobj == 'samehost': + # (sort samehost second after local) + return 129 + if sourceobj == 'samenet': + # Might write some fancy code to determine all prefix's + # from all interfaces and find a sane value for this one. + # For now, let's assume IPv4/24 or IPv6/96 (both have weight 96). + return 96 + if sourceobj[0] == '.': + # suffix matching (domain name), let's asume a very large scale + # and therefore a very low weight IPv4/16 or IPv6/64 (both have weight 64). + return 64 + # hostname, let's asume only one host matches, which is + # IPv4/32 or IPv6/128 (both have weight 128) + return 128 + raise PgHbaValueError('Cannot deduct the source weight of this source {1}'.format(sourceobj)) + + def source_type_weight(self): + """Give a weight on the type of this source. + + Basically make sure that IPv6Networks are sorted higher than IPv4Networks. + This is a 'when all else fails' solution in __lt__. + """ + sourceobj = self.source() + if isinstance(sourceobj, ipaddress.IPv4Network): + return 2 + if isinstance(sourceobj, ipaddress.IPv6Network): + return 1 + if isinstance(sourceobj, str): + return 0 + raise PgHbaValueError('This source {1} is of an unknown type...'.format(sourceobj)) + + def db_weight(self): + """Report the weight of the database. + + Normally, just 1, but for replication this is 0, and for 'all', this is more than 2. + """ if self['db'] == 'all': - dbweight = numdbs - elif self['db'] == 'replication': - dbweight = 0 - elif self['db'] in ['samerole', 'samegroup']: - dbweight = 1 - else: - dbweight = 1 + self['db'].count(',') + return 100000 + if self['db'] == 'replication': + return 0 + if self['db'] in ['samerole', 'samegroup']: + return 1 + return 1 + self['db'].count(',') + def user_weight(self): + """Report weight when comparing users.""" if self['usr'] == 'all': - uweight = numusers - else: - uweight = 1 - - ret = [] - for character in order: - if character == 'u': - ret.append(uweight) - elif character == 's': - ret.append(srcweight) - elif character == 'd': - ret.append(dbweight) - ret.append(sourceobj) - - return tuple(ret) + return 1000000 + return 1 def main(): diff --git a/test/integration/targets/postgresql/defaults/main.yml b/test/integration/targets/postgresql/defaults/main.yml index 2b6477921b..4085be3918 100644 --- a/test/integration/targets/postgresql/defaults/main.yml +++ b/test/integration/targets/postgresql/defaults/main.yml @@ -18,10 +18,20 @@ pg_hba_test_ips: - source: '192.168.0.0/24' netmask: '' databases: 'all,replication' +- source: '192.168.1.0/24' + netmask: '' + databases: 'all' + method: reject +- source: '127.0.0.1/32' + netmask: '' +- source: '::1/128' + netmask: '' - source: '0000:ff00::' netmask: 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ff00' + method: scram-sha-256 - source: '172.16.0.0' netmask: '255.255.0.0' + method: trust # defaults for test SSL ssl_db: 'ssl_db' diff --git a/test/integration/targets/postgresql/tasks/postgresql_pg_hba.yml b/test/integration/targets/postgresql/tasks/postgresql_pg_hba.yml index 2bb6a8e3ab..2d99dc0aa6 100644 --- a/test/integration/targets/postgresql/tasks/postgresql_pg_hba.yml +++ b/test/integration/targets/postgresql/tasks/postgresql_pg_hba.yml @@ -10,7 +10,7 @@ source: '0000:ffff::' netmask: 'ffff:fff0::' method: md5 - backup: true + backup: 'True' order: sud state: "{{item}}" check_mode: yes @@ -28,7 +28,7 @@ contype: "{{item.contype|default('host')}}" databases: "{{item.databases|default('all')}}" dest: /tmp/pg_hba.conf - method: md5 + method: "{{item.method|default('md5')}}" netmask: "{{item.netmask|default('')}}" order: sud source: "{{item.source|default('')}}" @@ -44,12 +44,12 @@ - name: Add several ip addresses postgresql_pg_hba: - backup: true + backup: 'True' contype: "{{item.contype|default('host')}}" - create: true + create: 'True' databases: "{{item.databases|default('all')}}" dest: /tmp/pg_hba.conf - method: md5 + method: "{{item.method|default('md5')}}" netmask: "{{item.netmask|default('')}}" order: sud source: "{{item.source|default('')}}" @@ -68,7 +68,7 @@ contype: "{{item.contype|default('host')}}" databases: "{{item.databases|default('all')}}" dest: /tmp/pg_hba.conf - method: md5 + method: "{{item.method|default('md5')}}" netmask: "{{item.netmask|default('')}}" order: sud source: "{{item.source|default('')}}" @@ -84,7 +84,7 @@ - name: Add new ip address for backup check and netmask_sameas_prefix check postgresql_pg_hba: - backup: true + backup: 'True' contype: host dest: /tmp/pg_hba.conf method: md5 @@ -96,7 +96,7 @@ - name: Add new ip address for netmask_sameas_prefix check postgresql_pg_hba: - backup: true + backup: 'True' contype: host dest: /tmp/pg_hba.conf method: md5 @@ -122,17 +122,22 @@ register: pg_hba_fail_src_all_with_netmask ignore_errors: yes +- debug: + var: pg_hba.pg_hba - assert: that: - 'pg_hba.pg_hba == [ - { "db": "all", "method": "md5", "type": "local", "usr": "all" }, { "db": "all", "method": "md5", "type": "local", "usr": "postgres" }, - { "db": "all", "method": "md5", "src": "0:ff00::/120", "type": "host", "usr": "all" }, - { "db": "all", "method": "md5", "src": "192.168.0.0/24", "type": "host", "usr": "all" }, + { "db": "all", "method": "md5", "type": "local", "usr": "all" }, + { "db": "all", "method": "md5", "src": "127.0.0.1/32", "type": "host", "usr": "all" }, + { "db": "all", "method": "md5", "src": "::1/128", "type": "host", "usr": "all" }, + { "db": "all", "method": "scram-sha-256", "src": "0:ff00::/120", "type": "host", "usr": "all" }, { "db": "replication", "method": "md5", "src": "192.168.0.0/24", "type": "host", "usr": "all" }, - { "db": "all", "method": "md5", "src": "172.16.0.0/16", "type": "host", "usr": "all" }, + { "db": "all", "method": "md5", "src": "192.168.0.0/24", "type": "host", "usr": "all" }, + { "db": "all", "method": "reject", "src": "192.168.1.0/24", "type": "host", "usr": "all" }, + { "db": "all", "method": "trust", "src": "172.16.0.0/16", "type": "host", "usr": "all" }, { "db": "all", "method": "md5", "src": "0:fff0::/28", "type": "host", "usr": "all" } - ]' + ]' - 'pg_hba_change is changed' - 'pg_hba_checkmode_check.stat.exists == false' - 'not pg_hba_idempotency_check1 is changed'