On Mon, 11 Sept 2023 at 17:12, Stephen Frost <sfrost@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.