Re: Allow file inclusion in pg_hba and pg_ident files

Julien Rouhaud <rjuju123@gmail.com>

From: Julien Rouhaud <rjuju123@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Aleksander Alekseev <aleksander@timescale.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nathan Bossart <nathandbossart@gmail.com>
Date: 2022-07-08T06:57:21Z
Lists: pgsql-hackers
Hi,

On Thu, Jun 02, 2022 at 10:08:15AM +0900, Michael Paquier wrote:
> On Thu, May 26, 2022 at 03:26:57PM +0800, Julien Rouhaud wrote:
>
> > After a bit more digging, I think that this comes from the fact that there's no
> > "official" name for this file.  Even the documentation just says "the
> > pg_hba.conf file" [1].  So using pg_hba.conf can either means explicitly
> > $PGDATA/pg_hba.conf or the instance's HBA file in general, whatever its
> > location.
> >
> > I think it would be good to improve this, including in the doc, but I'm
> > assuming it's entirely for HEAD only, including the error messages?
>
> Yes, that would be a set of changes only for HEAD, once 16~ opens for
> business.  FWIW, the acronym "HBA" is defined as "Host-Based
> Authentication", so we could use that as a base for the description of
> the file, using simply HBA in the follow-up paragraphs for simplicity,
> telling that pg_hba.conf is the default.

Ok.

> > If so, should I also change the doc to replace "pg_hba.conf" with something
> > else when it's not referring to the file default name?
> >
> > I'm thinking of using "HBA file" to replace pg_hba.conf, and using
> > "authentication file" when it can be either the "HBA file" and the "User Name
> > Maps file", would that be ok?
>
> Using "HBA file" in the docs is fine by me, knowing that the acronym
> is already defined.  The modified parts of the docs should perhaps
> mention once something like "Host-Based Authentication file (or HBA
> file)" for clarity.  For the error message, I think that we tend to
> avoid those acronyms, don't we?

I don't have an extensive knowledge of all the user facing error messages, but
after a quick grep I see multiple usage of OID, PID, GIN and other defined
acronyms.  I also see multiple occurrences of "only heap AM is supported",
while AM isn't even a defined acronym.

It doesn't seem that my proposal would be inconsistent with existing messages
and will help to reduce the message length, but if you prefer to keep the full
name I'm fine with it.  Those should be very rare and specialized errors
anyway.

While on the bikeshedding part, are you ok with the proposed keywords (include
and include_dir), behaving exactly like for postgresql.conf, and to also add
include_if_exists, so that we have the exact same possibilities with
postgresql.conf, pg_hba.conf and pg_ident.conf?



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