Re: Possibility to disable `ALTER SYSTEM`

On Mon, 18 Mar 2024 at 18:27, Robert Haas <robertmhaas(at)gmail(dot)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 <hlinnaka(at)iki(dot)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 <hlinnaka(at)iki(dot)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)
>