pg_hosts: Add pg_hosts_file_rules()
Hello,
While working on the configuration file format proposal, I noticed
that we are missing pg_hosts_file_rules, the pg_hosts version of
pg_hba_file_rules.
The attached patch adds this functionality, mirroring how the hba
version is implemented.
One detail I am definitely unsure about is the first rule_number
column, since pg_hosts is explicitly unordered, we might not want to
add it. I included it in 0001 for parity with hba_file_rules and
completeness.
Attachments:
0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-configu.patchapplication/octet-stream; name=0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-configu.patchDownload+359-4
On 8 Jul 2026, at 18:03, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
While working on the configuration file format proposal, I noticed
that we are missing pg_hosts_file_rules, the pg_hosts version of
pg_hba_file_rules.The attached patch adds this functionality, mirroring how the hba
version is implemented.
Thanks for your patch!
I think it's a good idea to add this. One thing I have been puzzling over is
what to do with LibreSSL builds which don't support ssl_sni. One could argue
that parsing the file doesn't require ssl_sni actually function, so the view
could still work, but on the other hand it might be confusing to users to learn
that their file doesn't generate any errors yet ssl_sni doesn't work. I think
I prefer if the view is blank and the file isn't parsed in clusters which
doesn't support ssl_sni. What are your thoughts on that?
One detail I am definitely unsure about is the first rule_number
column, since pg_hosts is explicitly unordered, we might not want to
add it. I included it in 0001 for parity with hba_file_rules and
completeness.
Consistency is good, but since the number in this view doesn't convey the same
meaning as the corresponding column for hba parsing I think we should omit it.
It adds no value on top of consistency and can at worst be misleading.
A few small comments on the patch:
+ The view <structname>pg_hosts_file_rules</structname> provides a summary of
+ the contents of the hosts configuration file, which maps incoming TLS
+ server name indication (SNI) host names to SSL certificate configuration.
I think this has too many moving parts to not have links to the corresponding
doc sections in order to help the reader. I propose something like the below
instead:
The view <structname>pg_hosts_file_rules</structname> provides a
summary of the contents of the <link linkend="hosts_file">hosts
configuration file</link>, which maps incoming TLS connections with
<xref="sslsni"/> set to SSL certificate configurations.
+# A malformed entry shows up as an error row, not a thrown error.
+$node->append_conf('pg_hosts.conf', "this line has too many fields a b c d e f");
+$node->reload;
+my $errs = $node->safe_psql('postgres',
+ "SELECT count(*) FROM pg_hosts_file_rules() WHERE error IS NOT NULL");
+ok($errs >= 1, "pg_hosts_file_rules: malformed conf entry becomes an error row");
This test doesn't need the ->reload command here does it, since the view parses
whats on the filesystem and not what's running. Also, since we know the view
shall have a single row with an error we should test for == 1 instead of >= 1.
Similarily, the previous test doesn't need to restart the cluster?
--
Daniel Gustafsson
I think it's a good idea to add this. One thing I have been puzzling over is
what to do with LibreSSL builds which don't support ssl_sni. One could argue
that parsing the file doesn't require ssl_sni actually function, so the view
could still work, but on the other hand it might be confusing to users to learn
that their file doesn't generate any errors yet ssl_sni doesn't work. I think
I prefer if the view is blank and the file isn't parsed in clusters which
doesn't support ssl_sni. What are your thoughts on that?
I agree, I was already thinking about this in more general ways as
part of the config rework (what else could be in pg_hosts), but for
the context of this patch it's better to limit it similarly to the
main feature.
Should I also try to add a liressl specific test to it?
I also addressed the other comments, except:
This test doesn't need the ->reload command here does it, since the view parses
whats on the filesystem and not what's running. Also, since we know the view
shall have a single row with an error we should test for == 1 instead of >= 1.
Similarily, the previous test doesn't need to restart the cluster?
The first is needed, or we need at least start, as postgres is stopped
before it (previous test fails the restart).
Attached v2.
Attachments:
v2-0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-conf.patchapplication/octet-stream; name=v2-0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-conf.patchDownload+348-4
On 14 Jul 2026, at 17:32, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
I think it's a good idea to add this. One thing I have been puzzling over is
what to do with LibreSSL builds which don't support ssl_sni. One could argue
that parsing the file doesn't require ssl_sni actually function, so the view
could still work, but on the other hand it might be confusing to users to learn
that their file doesn't generate any errors yet ssl_sni doesn't work. I think
I prefer if the view is blank and the file isn't parsed in clusters which
doesn't support ssl_sni. What are your thoughts on that?I agree, I was already thinking about this in more general ways as
part of the config rework (what else could be in pg_hosts), but for
the context of this patch it's better to limit it similarly to the
main feature.
+1
Should I also try to add a liressl specific test to it?
If you can find a good way to do it without convoluting the testfile which
currently just does a skip_all in case of LibreSSL, then why not. If it reduce
readability of the test file then it's probably not worth it since it's not
using any different codepaths from the OpenSSL counterpart.
This test doesn't need the ->reload command here does it, since the view parses
whats on the filesystem and not what's running. Also, since we know the view
shall have a single row with an error we should test for == 1 instead of >= 1.
Similarily, the previous test doesn't need to restart the cluster?The first is needed, or we need at least start, as postgres is stopped
before it (previous test fails the restart).
Ah, of course. However, it would be nice to avoid another restart in order to
shave some time off the testrun. Since these tests aren't really affecting the
cluster state, how about placing them first in the pg_hosts.conf section
starting at at line 98. That way we can test various queries against this view
when the cluster is running before we start with tests that load pg_hosts.conf.
It also makes more sense to me to first test parsing, and then parsing+loading.
--
Daniel Gustafsson
If you can find a good way to do it without convoluting the testfile which
currently just does a skip_all in case of LibreSSL, then why not. If it reduce
readability of the test file then it's probably not worth it since it's not
using any different codepaths from the OpenSSL counterpart.
Done. I added an if with an early exit instead of the skip, it seems
to work properly with a local LibreSSL build I tested and I think it
looks okay.
It also makes more sense to me to first test parsing, and then parsing+loading.
Also done. I also modified the test to use a connection string so that
I don't have to add a trust entry to pg_hba before the connection
tests.
Attachments:
v3-0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-conf.patchapplication/octet-stream; name=v3-0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-conf.patchDownload+368-9
Hi,
I tested the latest v3 patch on PostgreSQL 20devel.
The patch applied cleanly, and the build and installation completed
without any issues. Since the patch adds a new built-in function, I
created a fresh cluster with initdb before testing.
Before applying the patch, pg_hosts_file_rules() was not available,
and calling it resulted in a "function does not exist" error.
After applying the patch, rebuilding, and starting the new cluster, I
confirmed that the pg_hosts_file_rules() function is present. Both \df
pg_hosts_file_rules and SELECT * FROM pg_hosts_file_rules(); worked as
expected. With an empty pg_hosts.conf, the function returned zero
rows, which is the expected behavior. I also verified that the
pg_hosts_file_rules view works correctly.
I didn't notice any issues during my testing.
Regards,
solai
On 16 Jul 2026, at 00:10, Zsolt Parragi <zsolt.parragi@percona.com> wrote:
If you can find a good way to do it without convoluting the testfile which
currently just does a skip_all in case of LibreSSL, then why not. If it reduce
readability of the test file then it's probably not worth it since it's not
using any different codepaths from the OpenSSL counterpart.Done. I added an if with an early exit instead of the skip, it seems
to work properly with a local LibreSSL build I tested and I think it
looks okay.
Makes sense.
+ "SELECT count(*) FROM pg_hosts_file_rules() WHERE error IS NULL",
One thing I noticed (and had missed before) is that we should SELECT from the
view and not the function.
It also makes more sense to me to first test parsing, and then parsing+loading.
Also done. I also modified the test to use a connection string so that
I don't have to add a trust entry to pg_hba before the connection
tests.
LGTM. While at it I think we should take the opportunity to test selecting
from the view with the file missing before we populate it and test parsing. It
will be a cheap test to add and will cover one more path.
One small nit wit the LibreSSL hunk in the patch:
../src/backend/utils/adt/hbafuncs.c:467:1: warning: unused function 'fill_hosts_line' [-Wunused-function]
fill_hosts_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
^
1 warning generated.
With those two small nitpicks addressed I think this patch is pretty much ready for v20.
--
Daniel Gustafsson
One thing I noticed (and had missed before) is that we should SELECT from the
view and not the function.
I keep referring to it by the function name everywhere, even in the
commit message... fixed, it's now only used for the view definition.
I also added ifdefs around fill_hosts_line.
Attachments:
v4-0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-conf.patchapplication/octet-stream; name=v4-0001-Add-pg_hosts_file_rules-to-inspect-the-hosts-conf.patchDownload+377-9
Attached v5 as a CI fix for EXEC_BACKEND builds: there pg_hosts
changes are active immediately, and that caused the verification
connecion to fail. Now the tests are before we enable SNI, so work
properly even there.