Re: Allow file inclusion in pg_hba and pg_ident files

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>
Subject: Re: Allow file inclusion in pg_hba and pg_ident files
Date: 2022-07-19 06:13:12
Message-ID: YtZLeJPlMutL9heh@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-07-19 06:37:42 Re: [PoC] Let libpq reject unexpected authentication requests
Previous Message Kyotaro Horiguchi 2022-07-19 06:04:27 Re: fix stats_fetch_consistency value in postgresql.conf.sample