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: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>
Date: 2022-07-19T06:13:12Z
Lists: pgsql-hackers 
On Mon, Jul 18, 2022 at 03:11:51PM +0800, Julien Rouhaud wrote:
> So first, even if we can test 99% of the features with just testing the views
> output, I think it's should use the TAP framework since the tests will have to
> mess with the pg_ident/pg_hba files.  It's way easier to modify the auth files,
> and it uses a dedicated instance so we don't have to worry about breaking other
> test that would run concurrently.

Agreed.

> That may still be workable by splitting the output per newline (and possibly
> removing the first prompt before sending the query text), and remove the first
> and last entry (assuming you want to test somewhat sane data, and not e.g. run
> the regression test on a database containing a newline), but then you have to
> also account for possible escape sequences, for instance if you use
> enable-bracketed-paste.  In that case, the output becomes something like
> 
> dbname=# SELECT 1;
> [?2004l
> 1
> [?2004hpostgres=#
> 
> It could probably be handled with some regexp to remove escape sequences and
> remove empty lines, but it seems like really fragile, and thus a very bad idea.

Hmm.  Indeed, that sounds fragile.  And I don't really think that we
should make more testing infrastructure a requirement for this patch
as being able to maintain a connection while running the tests is
something we've bumped on for a bit of time.

> I'm not really sure what should be done here.  The best compromise I can think
> of is to split the tests in 3 parts:
> 
> 1) view reporting with various inclusions using safe_psql()

You mean in the case where the HBA and indent files can be loaded,
so as it is possible to peek at the system views without the
EXEC_BACKEND problem, right?

> 2) log error reporting

This one should be reliable and stable enough by parsing the logs of
the backend, thanks to connect_ok() and connect_fails().

> 3) view reporting with various inclusions errors, using safe_psql()
> 
> And when testing 3), detect first if we can still connect after introducing
> errors.  If not, assume this is Windows / EXEC_BACKEND and give up here without
> reporting an error.  Otherwise continue, and fail the test if we later can't
> connect anymore.  As discussed previously, detecting that the build is using
> the fork emulation code path doesn't seem worthwhile so guessing it from the
> psql error may be a better approach.

Yeah, we could do that.  Now we may also fail on other patterns, so we
would need to make sure that a set of expected error outputs are the
ones generated?  I'd be fine to give up testing the error output
generated in the system views at the end.  Without a persistent
connection state with the same kind of APIs as any of the drivers able
to do so, that's going to be a PITA to maintain.
--
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