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-01T11:28:09Z
Lists: pgsql-hackers 
On Tue, Mar 01, 2022 at 05:19:50PM +0800, Julien Rouhaud wrote:
> On Tue, Mar 01, 2022 at 04:45:48PM +0900, Michael Paquier wrote:
>> 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?
> 
> Yes, I followed the file convention to put the static functions first and then
> the exposed functions, and git-diff makes a terrible mess out of it :(

I'd like to think that not doing such a thing would be more helpful in
this case.  As the diffs show, anyone is going to have a hard time to
figure out if there are any differences in any of those routines, and
if these are the origin of a different problem.  A second thing is
that this is going to make back-patching unnecessarily harder.

> There's no functional change apart from exposing some functions and moving some
> in another file, so I though it's still ok to keep some consistency.  There
> isn't much changes backpatched in that file, so it shouldn't create more
> maintenance burden than simply splitting the file.

A lot of files do that already.  History clarity matters most IMO.

> As I mentioned in my initial email, I intentionally didn't add any test in the
> patchset yet, except the exact same coverage for the new view as there's for
> pg_hba_file_rules.  Ideally I'd like to add tests only once, to cover both 002
> and 0003.  But I don't want to waste time for that right now, especially since
> no one seems to be interested in 0003.

But that would be helpful for 0002.  I think that we should have a bit
more coverage in this area.  pg_hba_file_rules could gain in coverage,
additionally, but this is unrelated to what you are proposing here..

>> Does this pass
>> on Windows where pg_regress sets some mappings for SSPI when creating
>> one or more roles?
> 
> According to CI and cfbot yes.  E.g.
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/37/3558.
> Note that the failed runs are the warning I mentioned for mingw32 and the POC
> 0004, which is now fixed.

Interesting, I would not have expected that.  I may poke at that more
seriously.
--
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