Date: Fri, 27 Feb 2015 11:59:50 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Providing catalog view to pg_hba.conf file - Patch
 submission
Message-ID: <20150227165950.GP29780@tamriel.snowman.net>
References: 
 <CABUevEwzKy5JR8papHw5Amk=Ok6uddYtwQ+gJntmLdrx2wOgbA@mail.gmail.com>
 <82897A1301080E4B8E461DDAA0FFCF14DDE1@SYD1216>
 <82897A1301080E4B8E461DDAA0FFCF14FD6E@SYD1216>
 <20140629165554.GA5984@toroid.org>
 <20140630070646.GY31357@toroid.org>
 <CAJrrPGe3sZWKFfsHDCJMXfBOqkf6_w6R_DpsUYn2vQK_tVeU1g@mail.gmail.com>
 <54C8156E.1080500@BlueTreble.com>
 <CAJrrPGcFk9_svvipFJeLBrVJYOmq6uNy38NsT8jtfo6Bks5RYw@mail.gmail.com>
 <54C95C4B.2080901@BlueTreble.com>
 <54F09D68.2090700@2ndquadrant.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
 protocol="application/pgp-signature"; boundary="PEWkPmBCIbC5Tcj0"
Content-Disposition: inline
In-Reply-To: <54F09D68.2090700@2ndquadrant.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--PEWkPmBCIbC5Tcj0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

All,

* Tomas Vondra (tomas.vondra@2ndquadrant.com) wrote:
> On 28.1.2015 23:01, Jim Nasby wrote:
> > On 1/28/15 12:46 AM, Haribabu Kommi wrote:
> >>> >Also, what happens if someone reloads the config in the middle of
> >>> running
> >>> >the SRF?
> >> hba entries are reloaded only in postmaster process, not in every
> >> backend.
> >> So there shouldn't be any problem with config file reload. Am i
> >> missing something?
> >=20
> > Ahh, good point. That does raise another issue though... there might
> > well be some confusion about that. I think people are used to the
> > varieties of how settings come into effect that perhaps this isn't an
> > issue with pg_settings, but it's probably worth at least mentioning in
> > the docs for a pg_hba view.
>=20
> I share this fear, and it's the main problem I have with this patch.

Uh, yeah, agreed.

> Currently, the patch always does load_hba() every time pg_hba_settings
> is accessed, which loads the current contents of the config file into
> the backend process, but the postmaster still uses the original one.
> This makes it impossible to look at currently effective pg_hba.conf
> contents. Damn confusing, I guess.

Indeed.  At a *minimum*, we'd need to have some way to say "what you're
seeing isn't what's actually being used".

> Also, I dare to say that having a system view that doesn't actually show
> the system state (but contents of a config file that may not be loaded
> yet), is wrong.

Right, we need to show what's currently active in PG-land, not just spit
back whatever the on-disk contents currently are.

> Granted, we don't modify pg_hba.conf all that often, and it's usually
> followed by a SIGHUP to reload the contents, so both the backend and
> postmaster should see the same stuff. But the cases when I'd use this
> pg_hba_settings view usually turned out to be 'forgot to SIGHUP', so
> this would not help, actually.

Agreed.

> What I can imagine is keeping the original list (parsed by postmaster,
> containing the effective pg_hba.conf contents), and keeping another list
> of 'current contents' within backends. And having a 'reload' flag for
> pg_hba_settings, determining which list to use.
>=20
>   pg_hba_settings(reload=3Dfalse) -> old list (from postmaster)
>   pg_hba_settings(reload=3Dtrue)  -> new list (from backend)
>=20
> The pg_hba_settings view should use 'reload=3Dfalse' (i.e. show effective
> contents of the hba).

I don't think we actually care what the "current contents" are from the
backend's point of view- after all, when does an individual backend ever
use the contents of pg_hba.conf after it's up and running?  What would
make sense, to me at least, would be:

pg_hba_configured() -- spits back whatever the config file has
pg_hba_active() -- shows what the postmaster is using currently

Or something along those lines.  Note that I'd want pg_hba_configured()
to throw an error if the config file can't be parsed (which would be
quite handy to make sure that you didn't goof up the config..).  This,
combined with pg_reload_conf() would provide a good way to review
changes before putting them into place.

> The other feature that'd be cool to have is a debugging function on top
> of the view, i.e. a function pg_hba_check(host, ip, db, user, pwd)
> showing which hba rule matched. But that's certainly nontrivial.

I'm not sure that I see why, offhand, it'd be much more than trivial..

	Thanks!

		Stephen

--PEWkPmBCIbC5Tcj0
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJU8KKGAAoJEO1sijiDR2RVIoIQAJEM5+wEyi1IialhBOrHwFhy
0PtoQWy3Eb3eIhGAQDdsmnjgxWO+7N18nY+/Ue9nW6zTkqT7ftkd6mHBZ1Q0Hizg
5AOaz20W+tPJ1+35LLAecA/tub7MDYsP5faAGwjU2Vlf/m2WHwDQmZoXIVXUT6Fm
vsRg+GsrzlcguEUDxA6RxKR9zhUzcw2L5LfwlwnK25K/oHV2oeoyf9zv81lfaZMP
SnZq/EJ7ICBod9vA3sRk5PCinl9qRbs3OykvDeamJPfa9NAtPKvEjWhnpZrizggE
sSXqGyrKdGg6CM5CuNKwGxPGquVIcblBw07EzMQ5EkdxL2bcpbj9AX9PZb6YvFnL
w4sqQ/mHq86WHh07pZhEFDYRsEiQ6seIqtgVt0o4YWVVlkQDUlAv8JBE1E8vk2p7
O6KW6Z+nfYQZWMEjeT0bUHrXUmVbS+1IOET0AvnnBiUqKwrI5DiMBkkGdx8rCKnD
Kw2+QE45wLyJb1bShjwJE980EPrrRqyIac3ZVj4bARsW1QoIeXS4X3xq0QFc/n0C
W+tIeFis4LaSMxr5INttqdOFFVwWetYM4BfbLtmoQuugoPT/QkW7aWmxPVZ6AIz4
AH0Ov0r2BOZHg20WEeft2vT5tI2rBYbEVfOIhU9nvgGhmkbYa5UmtZ7HBA53+1a5
GX0aefrTs7hnLQ2Q+g6c
=oXR0
-----END PGP SIGNATURE-----

--PEWkPmBCIbC5Tcj0--