Hi Stephen, On Mon, 11 Sept 2023 at 17:12, Stephen Frost <sfr...@snowman.net> wrote:
> A lot of the objections seem to be on the grounds of returning a > 'permission denied' kind of error and I generally agree with that being > the wrong approach. > > As an alternative idea- what if we had something in postgresql.conf > along the lines of: > > include_alter_system = true/false > > and use that to determine if the postgresql.auto.conf is included or > not..? > That sounds like a very good idea. I had thought about that when writing the PoC, as a SIGHUP controlled GUC. I had trouble finding an adequate GUC category for that (ideas?), and thought it was a more intrusive patch to trigger the conversation. But I am willing to explore that too (which won't change by any inch the goal of the patch). With the above, we could throw a WARNING or maybe just NOTICE when ALTER > SYSTEM is run that 'include_alter_system is false and therefore these > changes won't be included in the running configuration' or similar. > That's also an option I'd be willing to explore with folks here. > What this does cause problems with is that pg_basebackup and other tools > (eg: pgbackrest) write into postgresql.auto.conf currently and we'd want > those to still work. That's an opportunity, imv, though, since I don't > really think where ALTER SYSTEM writes to and where backup/restore > tools are writing to should really be the same place anyway. Therefore, > perhaps we add a 'postgresql.system.conf' or similar and maybe a > corresponding option in postgresql.conf to include it or not. > Totally. We are for example in the same position with the CloudNativePG operator, and it is something we are intending to fix ( https://github.com/cloudnative-pg/cloudnative-pg/issues/2727). I agree with you that it is the wrong place to do it. This is an issue if you're looking at it as a security thing. This > isn't an issue if don't view it that way. Still, I do see some merit in > making it so that you can't actually change the config that's loaded at > system start from inside the data directory or as the PG superuser, > which my proposal above would support- just configure in postgresql.conf > to not include any of the alter-system or generated config. The actual > postgresql.conf could be owned by root then too. > +1. Thank you, Gabriele -- Gabriele Bartolini Vice President, Cloud Native at EDB enterprisedb.com
