Re: [PATCH] HINT: pg_hba.conf changed since last config reload

From: Steve Singer <steve(at)ssinger(dot)info>
To: Craig Ringer <craig(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] HINT: pg_hba.conf changed since last config reload
Date: 2014-10-19 18:17:45
Message-ID: BLU437-SMTP21E9EF68C5F2F71312C1A2DC960@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 10/16/2014 11:34 PM, Craig Ringer wrote:
> Given the generally positive reception to this, here's a patch.
> The first patch adds an errhint_log , akin to the current errdetail_log,
> so we can send a different HINT to the server log than we do to the client.

The patch behaves as you describe. I feel that this feature would be
useful , and you implemented the suggestions given that requested the
reload notice but be sent to the client but instead just a hint about
checking the server log.

You follow the pattern set with detail_log which makes sense. The
variable name "hint_log" doesn't make it obvious to me that
the hint goes to the server log, but not the client. The comment for
errhint_log should maybe explicitly say that.

One question about the code:

Does errfinish (elog.c at around line 505) need to free hint_log ? (I
would assume it does)

Other than that the patch looks good to me.


Something else I noticed while testing. This isn't introduced by your
patch but I am wondering if it an existing bug if I setup my
configuration like this:

#data_directory = 'ConfigDir' # use data in another directory
# (change requires restart)
hba_file = 'ConfigDir/pg_hba2.conf' # host-based authentication file

and start postgres like

./postgres -D ../data

it looks for pg2hba2.conf at bin/ConfigDir/pg_hba2.conf (relative to
the bin directory I started it from)

Then if I change my pg_hba.conf and do a reload I get the following in
the log

LOG: parameter "hba_file" cannot be changed without restarting the server
LOG: configuration file
"/usr/local/pgsql95git/bin/../data/postgresql.conf" contains errors;
unaffected changes were applied

set_config_option is comparing the relative path with the absolute path.


> (Even if DETAIL was appropriate for this info, which it isn't, I can't
> use errdetail_log because it's already used for other information in
> some of the same error sites.)
> The second patch adds a test during errors to report if pg_hba.conf is
> stale, or if pg_ident.conf is stale.
> Typical output, client:
> psql: FATAL: Peer authentication failed for user "fred"
> HINT: See the server error log for additional information.
> Typical output, server:
> LOG: provided user name (fred) and authenticated user name (craig) do
> not match
> FATAL: Peer authentication failed for user "fred"
> DETAIL: Connection matched pg_hba.conf line 84: "local all
> all peer"
> HINT: pg_hba.conf has been changed since last server configuration
> reload. Reload the server configuration to apply the changes.
> I've added this to the next CF.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-10-19 18:31:13 pg_dump/pg_restore seem broken on hamerkop
Previous Message Magnus Hagander 2014-10-19 17:10:10 Re: [PATCH] add ssl_protocols configuration option