On Mon, 18 Mar 2024 at 18:27, Robert Haas <robertmh...@gmail.com> wrote: > I think for now we > should just file this under "Other platforms and clients," which only > has one existing setting. If the number of settings of this type > grows, we can split it out.
Done. I also included a patch to rename COMPAT_OPTIONS_CLIENTS to COMPAT_OPTIONS_OTHER, since that enum variant naming doesn't match the new intent of the section. On Tue, 19 Mar 2024 at 10:26, Heikki Linnakangas <hlinn...@iki.fi> wrote: > (Another way to read this proposal is to rename the GUC that's been > discussed in this thread to 'configuration_managed_externally'. That > makes it look less like a security feature, and describes the intended > use case.) I like this idea of naming the GUC in such a way. I swapped the words a bit and went for externally_managed_configuration, since order matches other GUCs e.g. standard_conforming_strings. But if you feel strongly about the ordering of the words, I'm happy to change it back. For the errorcode I now went for ERRCODE_FEATURE_NOT_SUPPORTED, which seemed most fitting. On Tue, 19 Mar 2024 at 10:26, Heikki Linnakangas <hlinn...@iki.fi> wrote: > > I want to remind everyone of this from Gabriele's first message that > started this thread: > > > At the moment, a possible workaround is that `ALTER SYSTEM` can be blocked > > by making the postgresql.auto.conf read only, but the returned message is > > misleading and that’s certainly bad user experience (which is very > > important in a cloud native environment): > > > > > > ``` > > postgres=# ALTER SYSTEM SET wal_level TO minimal; > > ERROR: could not open file "postgresql.auto.conf": Permission denied > > ``` > > I think making the config file read-only is a fine solution. If you > don't want postgres to mess with the config files, forbid it with the > permission system. > > Problems with pg_rewind, pg_basebackup were mentioned with that > approach. I think if you want the config files to be managed outside > PostgreSQL, by kubernetes, patroni or whatever, it would be good for > them to be read-only to the postgres user anyway, even if we had a > mechanism to disable ALTER SYSTEM. So it would be good to fix the > problems with those tools anyway. > > The error message is not great, I agree with that. Can we improve it? > Maybe just add a HINT like this: > > postgres=# ALTER SYSTEM SET wal_level TO minimal; > ERROR: could not open file "postgresql.auto.conf" for writing: > Permission denied > HINT: Configuration might be managed outside PostgreSQL > > > Perhaps we could make that even better with a GUC though. I propose a > GUC called 'configuration_managed_externally = true / false". If you set > it to true, we prevent ALTER SYSTEM and make the error message more > definitive: > > postgres=# ALTER SYSTEM SET wal_level TO minimal; > ERROR: configuration is managed externally > > As a bonus, if that GUC is set, we could even check at server startup > that all the configuration files are not writable by the postgres user, > and print a warning or refuse to start up if they are. > > (Another way to read this proposal is to rename the GUC that's been > discussed in this thread to 'configuration_managed_externally'. That > makes it look less like a security feature, and describes the intended > use case.) > > -- > Heikki Linnakangas > Neon (https://neon.tech) >
v6-0002-Add-externally_managed_configuration-GUC.patch
Description: Binary data
v6-0001-Rename-COMPAT_OPTIONS_CLIENT-to-COMPAT_OPTIONS_OT.patch
Description: Binary data