Horton

Re: Possibility to disable `ALTER SYSTEM`

Heikki Linnakangas <hlinnaka@iki.fi>

View thread
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>, Daniel Gustafsson <daniel@yesql.se>, Bruce Momjian <bruce@momjian.us>, Tom Lane <tgl@sss.pgh.pa.us>, Joel Jacobson <joel@compiler.org>, Andrew Dunstan <andrew@dunslane.net>, Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>, Magnus Hagander <magnus.hagander@redpill-linpro.com>, Maciek Sakrejda <m.sakrejda@gmail.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-03-19T09:26:20Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add allow_alter_system GUC.

  2. Rename COMPAT_OPTIONS_CLIENT to COMPAT_OPTIONS_OTHER.

  3. Remove support for version-0 calling conventions.

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)