pg_activity
pg_activity copied to clipboard
dalibo
Some of the new header information are not accessible in --rds mode.
Example:
psycopg2.errors.InsufficientPrivilege: permission denied for function pg_ls_tmpdir
Several options :
- require the
GRANTon the function orSUPERUSERorpg_read_server_files. - Check if
%(using_rds)sis equal to true in thetempfilesCTE - Check if we are superuser with
current_setting('is_superuser')::bool
3 seems way better but postgres doesn't like it (its friday maybe my brain is not in proper working condition):
[local]:5435 jack@postgres=> SELECT current_setting('is_superuser')::bool;
current_setting
-----------------
f
(1 row)
[local]:5435 jack@postgres=>
SELECT count(*) as temp_files,
COALESCE(SUM(size), 0) AS temp_bytes
FROM pg_tablespace ts, LATERAL pg_catalog.pg_ls_tmpdir(ts.oid) tmp
WHERE spcname <> 'pg_global'
AND current_setting('is_superuser')::bool;
ERROR: permission denied for function pg_ls_tmpdir
[local]:5435 jack@postgres=>
SELECT count(*) as temp_files,
COALESCE(SUM(size), 0) AS temp_bytes
FROM pg_tablespace ts, LATERAL pg_catalog.pg_ls_tmpdir(ts.oid) tmp
WHERE spcname <> 'pg_global'
AND false;
temp_files | temp_bytes
------------+------------
0 | 0
(1 row)
Turns out it's a very bad idea to do 3 like this because the result depends on what the planner decides :
[local]:5435 postgres@postgres=# EXPLAIN (analyze)
SELECT count(*) as temp_files,
COALESCE(SUM(size), 0) AS temp_bytes
FROM pg_tablespace ts, LATERAL pg_catalog.pg_ls_tmpdir(ts.oid) tmp
WHERE spcname <> 'pg_global'
AND current_setting('is_superuser')::bool;
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------
Aggregate (cost=2.07..2.08 rows=1 width=40) (actual time=0.205..0.208 rows=1 loops=1)
-> Result (cost=0.03..1.87 rows=40 width=8) (actual time=0.197..0.199 rows=0 loops=1)
One-Time Filter: (current_setting('is_superuser'::text))::boolean
-> Nested Loop (cost=0.03..1.87 rows=40 width=8) (actual time=0.186..0.188 rows=0 loops=1)
-> Seq Scan on pg_tablespace ts (cost=0.00..1.04 rows=2 width=4) (actual time=0.018..0.023 rows=2 loops=1)
Filter: (spcname <> 'pg_global'::name)
Rows Removed by Filter: 1
-> Function Scan on pg_ls_tmpdir tmp (cost=0.03..0.23 rows=20 width=8) (actual time=0.078..0.079 rows=0 loops=2)
Planning Time: 1.747 ms
Execution Time: 0.444 ms
(10 rows)
[local]:5435 postgres@postgres=# EXPLAIN (analyze)
SELECT count(*) as temp_files,
COALESCE(SUM(size), 0) AS temp_bytes
FROM pg_tablespace ts, LATERAL pg_catalog.pg_ls_tmpdir(ts.oid) tmp
WHERE spcname <> 'pg_global'
AND false;
QUERY PLAN
------------------------------------------------------------------------------------------
Aggregate (cost=0.00..0.01 rows=1 width=40) (actual time=0.007..0.009 rows=1 loops=1)
-> Result (cost=0.00..0.00 rows=0 width=8) (actual time=0.002..0.002 rows=0 loops=1)
One-Time Filter: false
Planning Time: 0.289 ms
Execution Time: 0.054 ms
(5 rows)
Just found another bug. This one is old.
It's not possible to run pg_database_size() on a database you can't connect (GRANT)to you re not SUPERUSER.
The tool probably doesn't work on rds in that case.
From the docs of pg_database_size :
Computes the total disk space used by the database with the specified name or OID. To use this function, you must have CONNECT privilege on the specified database (which is granted by default) or be a member of the pg_read_all_stats role.
permission denied for function
After a few tests, I think that checking if the user is superuser is not the right move because across versions some checks where relaxed so that non superuser with the correct permissions can use some functions (e.g. pg_read_server_files or grant execute on some functions). It becomes really difficult to check who can do what.
On the other hand we have the "--rds" options, it's designed to :
- not execute things that should / could require special / superuser privilege
- filter out the "rds_admin" database
The filtering part is redundant with the new filter feature.
I propose that we :
- add a "--unprivileged-user" / "--no-superuser" option, and use it to disable parts of the queries that require superuser or additional permissions
- mark the "--rds" option as deprecated and suggest using the new option + the filter option to archive the same thing. (and do it that way internally)
the pg_database_size() & CONNECT GRANT
It's a mess to test this. I propose to just document that CONNECT must be explicitly granted to all the databases if CONNECT was revoked from public. If it's not possible the user should filter the database out.
@dlax: what do you think about this ?
- add a "--unprivileged-user" / "--no-superuser" option, and use it to disable parts of the queries that require superuser or additional permissions
If we go with requiring superuser, can't we check this with 'select rolsuper from pg_roles where rolname = current_user'?
Otherwise, if we can work with less permissions, that's even better IMO. Then the flag you propose might be useful unless we are able to document which permissions are exactly required depending on Postgres version (e.g. pg_read_server_files). But this documentation effort would be valuable anyway.
- mark the "--rds" option as deprecated and suggest using the new option + the filter option to archive the same thing. (and do it that way internally)
Rather do this internally, keeping the option should be cheap.
the pg_database_size() & CONNECT GRANT
It's a mess to test this. I propose to just document that CONNECT must be explicitly granted to all the databases if CONNECT was revoked from public. If it's not possible the user should filter the database out.
There is also --no-db-size which would skip pg_database_size() calls if I understand correctly. Or is it not enough?
If we go with requiring superuser, can't we check this with 'select rolsuper from pg_roles where rolname = current_user'?
It's not enough, on old versions SUPERUSER was required but lately they started to relax that kind of requirement and allow users to GRANT access to some previously restricted function directly of via generic roles (pg_* roles). Some users might not be able to have those grants.
Rather do this internally, keeping the option should be cheap.
Ok, I was slightly worried we would end up with --azure, --google etc..
There is also --no-db-size which would skip pg_database_size() calls if I understand correctly. Or is it not enough?
Yes but it will skip all the pg_database_size() calls. I suppose it's enough.
To clarify, I'll move forward with a "--unprivileged-user" option and adapt the "rds" mode to use this + the filter option.
An other option came up last night.
We could also make a --no-temp-size since it's the only thing requiring special rights right now. But as features add in we could end up with a slew of --no-* switches.
Ok, I was slightly worried we would end up with
--azure,
Ah, yes. Might be a good reason to drop the option then.
We could also make a
--no-temp-sizesince it's the only thing requiring special rights right now. But as features add in we could end up with a slew of--no-*switches.
If you think this is tractable, I agree this would be nicer.
There are now flags with --no-tempfiles and --no-walreceiver.
pg_actvity can now be used without superuser privilege.
hint message are displayed on exit to explain why a feature was disabled at runtime.
Last thing would be to use filters to implement the "rdsadmin" exclusion.
After checking the code, the --rds option filters out the rdsadmin database only for space computation.
It's therefore not possible to archive the same thing with filters.
I'll add a note in the man page an call it quits.