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: Aleksander Alekseev <aleksander(at)timescale(dot)com>, 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-06-02 01:08:15
Message-ID: YpgNfzLBV6mdtRSg@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 26, 2022 at 03:26:57PM +0800, Julien Rouhaud wrote:
> So you mean having an error message like that (having an "include myconf"
> in the HBA file):
>
> LOG: could not open authentication file "myconf" as "/path/to/myconf": No such file or directory
> LOG: pg_hba.conf was not reloaded
>
> This would be somewhat consistent with how it's done for the postgresql.conf,
> assuming that we do fix that hardcoded "pg_hba.conf":
>
> LOG: could not open configuration file "/path/to/toto": No such file or directory
> LOG: configuration file "/path/to/postgresql.conf" contains errors; no changes were applied
>
> With this message it's clear that the file that couldn't be opened is an
> included file.

Yes, consistency would be good here.

> However those two cases are slightly different, and technically the "secondary
> file" is not an authentication file, just a list of tokens, so I'm still a bit
> worried about ambiguity here.

Hmm. Yes, using only this term could lead to some confusion.
Thinking more about that, perhaps you are right to use fully separated
terms for the ident and HBA parts, particularly once the inclusion is
in place as the file names could be anything, so we would lose context
about where a file for used for what.

> 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.

> 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?

> Ah I see. Sure I could split it in another commit too, but this isn't
> backpatchable (and pg15 isn't branched yet anyway) so I'm not sure how useful
> that is. I will go head and do that anyway, it will be easy to merge the
> commits if needed.

The rule_number shines with the inclusion logic in place, still
splitting that into a separate commit makes the review and the commit
history cleaner IMO. Perhaps that's just me being overly-pedantic in
keeping a clean commit history :)
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Anton A. Melnikov 2022-06-02 01:22:52 [PATCH] Fix pg_upgrade test from v10
Previous Message Michael Paquier 2022-06-02 00:37:58 Re: pg_upgrade test writes to source directory