Horton

Re: Allow file inclusion in pg_hba and pg_ident files

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2022-03-01T07:45:48Z
Lists: pgsql-hackers 
On Mon, Feb 28, 2022 at 07:42:17PM +0800, Julien Rouhaud wrote:
> Done in attached v2.  I did the split in a separate commit, as the diff is
> otherwise unreadable.  While at it I also fixed a few minor issues (I missed a
> MemoryContextDelete, and now avoid relying on inet_net_pton which apparently
> doesn't exist in cygwin).

Hmm.  The diffs of 0001 are really hard to read.  Do you know why this
is happening?  Is that because some code has been moved around?  I
have been doing a comparison of all the routines showing up in the
diffs, to note that the contents of load_hba(), load_ident(),
hba_getauthmethod() & friends are actually the same, but this makes
the change history harder to follow.  Moving around fill_hba_line()
and fill_hba_view() should be enough, indeed.

+#include "utils/guc.h"
+//#include "utils/tuplestore.h"

Ditto.

+   /* Build a tuple descriptor for our result type */
+   if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+       elog(ERROR, "return type must be a row type");

Worth noting that I was planning to apply a patch from Melanie
Plageman to simplify the creation of tupledesc and tuplestores for
set-returning functions like this one, so this would cut a bit of code
here.  This is not directly related to your patch, though, that's my
business :)

Well, as of 0002, one thing that makes things harder to follow is that
parse_ident_line() is moved at a different place in hba.c, one slight
difference being err_msg to store the error message in the token
line..  But shouldn't the extension of parse_ident_line() with its
elevel be included in 0001?  Or, well, it could just be done in its
own patch to make for a cleaner history, so as 0002 could be shaped as
two commits itself.

Also, it seems to me that we'd better have some TAP tests for that to
make sure of its contents?  One place would be src/test/auth/.
Another place where we make use of user mapping is the krb5 tests but
these are run in a limited fashion in the buildfarm.  We also set some
mappings for SSPI on Windows all the time, so we'd better be careful
about that and paint some $windows_os in the tests when looking at the
output of the view.

+-- We expect no user mapping in this test
+select count(*) = 0 as ok from pg_ident_file_mappings;

It could be possible to do installcheck on an instance that has user
mappings meaning that this had better be ">= 0", no?  Does this pass
on Windows where pg_regress sets some mappings for SSPI when creating
one or more roles?
--
Michael

Commits

  1. Add TAP tests for include directives in HBA end ident files

  2. Introduce variables for initial and max nesting depth on configuration files

  3. Add support for file inclusions in HBA and ident configuration files

  4. Add missing initialization in tokenize_expand_file() for output list

  5. Rework memory contexts in charge of HBA/ident tokenization

  6. Add error context callback when tokenizing authentication files

  7. Invent open_auth_file() in hba.c to refactor authentication file opening

  8. Use AbsoluteConfigLocation() when building an included path in hba.c

  9. Move code related to configuration files in directories to new file

  10. doc: Fix some descriptions related to pg_ident_file_mappings

  11. Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings

  12. Refactor code handling the names of files loaded in hba.c

  13. Make consistent a couple of log messages when parsing HBA files

  14. Use hba_file/ident_file GUCs rather than pg_hba.conf/pg_ident.conf in logs

  15. Fix path reference when parsing pg_ident.conf for pg_ident_file_mappings

  16. Add system view pg_ident_file_mappings

  17. Modify query on pg_hba_file_rules to check for errors in regression tests

  18. Refactor code related to pg_hba_file_rules() into new file