Skip to content

Commit dafd472

Browse files
mkleczekmkleczek
committed
refactor: get rid of RoleSettings cache in Config
1 parent 955fd00 commit dafd472

8 files changed

Lines changed: 111 additions & 76 deletions

File tree

‎src/PostgREST/AppState.hs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ import PostgREST.Config.Database (queryDbSettings,
6363
queryPgVersion,
6464
queryRoleSettings)
6565
import PostgREST.Config.PgVersion (PgVersion (..),
66-
minimumPgVersion)
66+
minimumPgVersion,
67+
pgVersion150)
6768
import PostgREST.Debounce (makeDebouncer)
6869
import PostgREST.SchemaCache (SchemaCache (..),
6970
querySchemaCache,
@@ -406,17 +407,17 @@ readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
406407
Right x -> pure x
407408
else
408409
pure mempty
409-
(roleSettings, roleIsolationLvl) <-
410+
roleIsolationLvl <-
410411
if configDbConfig conf then do
411-
rSettings <- usePool appState (queryRoleSettings pgVer)
412+
rSettings <- usePool appState queryRoleSettings
412413
case rSettings of
413414
Left e -> do
414415
observer $ QueryRoleSettingsErrorObs e
415-
pure (mempty, mempty)
416+
pure mempty
416417
Right x -> pure x
417418
else
418419
pure mempty
419-
readAppConfig dbSettings (configFilePath conf) (Just $ configDbUri conf) roleSettings roleIsolationLvl >>= \case
420+
readAppConfig dbSettings (configFilePath conf) (Just $ configDbUri conf) (pgVer >= pgVersion150) roleIsolationLvl >>= \case
420421
Left err ->
421422
if startingUp then
422423
panic err -- die on invalid config if the program is starting up

‎src/PostgREST/CLI.hs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import Protolude
3030
main :: CLI -> IO ()
3131
main CLI{cliCommand, cliPath} = do
3232
conf <-
33-
either panic identity <$> Config.readAppConfig mempty cliPath Nothing mempty mempty
33+
either panic identity <$> Config.readAppConfig mempty cliPath Nothing False mempty
3434
case cliCommand of
3535
Client adminCmd -> runClientCommand conf adminCmd
3636
Run runCmd -> runAppCommand conf runCmd

‎src/PostgREST/Config.hs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,7 @@ import Numeric (readOct, showOct)
6161
import System.Environment (getEnvironment)
6262
import System.Posix.Types (FileMode)
6363

64-
import PostgREST.Config.Database (RoleIsolationLvl,
65-
RoleSettings)
64+
import PostgREST.Config.Database (RoleIsolationLvl)
6665
import PostgREST.Config.JSPath (FilterExp (..), JSPath,
6766
JSPathExp (..), dumpJSPath,
6867
pRoleClaimKey)
@@ -98,6 +97,7 @@ data AppConfig = AppConfig
9897
, configDbRootSpec :: Maybe QualifiedIdentifier
9998
, configDbSchemas :: NonEmpty Text
10099
, configDbConfig :: Bool
100+
, configDbHasParameterPrivilege :: Bool
101101
, configDbPreConfig :: Maybe QualifiedIdentifier
102102
, configDbTimezoneEnabled :: Bool
103103
, configDbTxAllowOverride :: Bool
@@ -126,7 +126,6 @@ data AppConfig = AppConfig
126126
, configAdminServerPort :: Maybe Int
127127
, configAdminServerUnixSocket :: Maybe FilePath
128128
, configAdminServerUnixSocketMode :: FileMode
129-
, configRoleSettings :: RoleSettings
130129
, configRoleIsoLvl :: RoleIsolationLvl
131130
, configInternalSCQuerySleepFst :: Maybe Int32
132131
, configInternalSCQuerySleepSnd :: Maybe Int32
@@ -250,13 +249,13 @@ instance JustIfMaybe a (Maybe a) where
250249

251250
-- | Reads and parses the config and overrides its parameters from env vars,
252251
-- files or db settings.
253-
readAppConfig :: [(Text, Text)] -> Maybe FilePath -> Maybe Text -> RoleSettings -> RoleIsolationLvl -> IO (Either Text AppConfig)
254-
readAppConfig dbSettings optPath prevDbUri roleSettings roleIsolationLvl = do
252+
readAppConfig :: [(Text, Text)] -> Maybe FilePath -> Maybe Text -> Bool -> RoleIsolationLvl -> IO (Either Text AppConfig)
253+
readAppConfig dbSettings optPath prevDbUri hasParameterPrivilege roleIsolationLvl = do
255254
env <- readPGRSTEnvironment
256255
-- if no filename provided, start with an empty map to read config from environment
257256
conf <- maybe (return $ Right M.empty) loadConfig optPath
258257

259-
case C.runParser (parser optPath env dbSettings roleSettings roleIsolationLvl) =<< mapLeft show conf of
258+
case C.runParser (parser optPath env dbSettings hasParameterPrivilege roleIsolationLvl) =<< mapLeft show conf of
260259
Left err ->
261260
return . Left $ "Error in config " <> err
262261
Right parsedConfig ->
@@ -273,8 +272,8 @@ readAppConfig dbSettings optPath prevDbUri roleSettings roleIsolationLvl = do
273272
readSecretFile =<<
274273
readDbUriFile prevDbUri parsedConfig
275274

276-
parser :: Maybe FilePath -> Environment -> [(Text, Text)] -> RoleSettings -> RoleIsolationLvl -> C.Parser C.Config AppConfig
277-
parser optPath env dbSettings roleSettings roleIsolationLvl =
275+
parser :: Maybe FilePath -> Environment -> [(Text, Text)] -> Bool -> RoleIsolationLvl -> C.Parser C.Config AppConfig
276+
parser optPath env dbSettings hasParameterPrivilege roleIsolationLvl =
278277
AppConfig
279278
<$> parseAppSettings "app.settings"
280279
<*> parseErrorVerbosity "client-error-verbosity"
@@ -300,6 +299,7 @@ parser optPath env dbSettings roleSettings roleIsolationLvl =
300299
(optString "root-spec"))
301300
<*> parseDbSchemas "db-schemas" "db-schema"
302301
<*> (fromMaybe True <$> optBool "db-config")
302+
<*> pure hasParameterPrivilege
303303
<*> (fmap toQi <$> optString "db-pre-config")
304304
<*> (fromMaybe True <$> optBool "db-timezone-enabled")
305305
<*> parseTxEnd "db-tx-end" snd
@@ -331,7 +331,6 @@ parser optPath env dbSettings roleSettings roleIsolationLvl =
331331
<*> parseAdminServerPort "admin-server-port"
332332
<*> (fmap T.unpack <$> optString "admin-server-unix-socket")
333333
<*> parseSocketFileMode "admin-server-unix-socket-mode"
334-
<*> pure roleSettings
335334
<*> pure roleIsolationLvl
336335
<*> optInt "internal-schema-cache-query-sleep-before-queries"
337336
<*> optInt "internal-schema-cache-query-sleep"

‎src/PostgREST/Config/Database.hs‎

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,12 @@ module PostgREST.Config.Database
55
, queryDbSettings
66
, queryPgVersion
77
, queryRoleSettings
8-
, RoleSettings
98
, RoleIsolationLvl
109
, TimezoneNames
1110
, toIsolationLevel
1211
) where
1312

14-
import Control.Arrow ((***))
15-
16-
import PostgREST.Config.PgVersion (PgVersion (..), pgVersion150)
13+
import PostgREST.Config.PgVersion (PgVersion (..))
1714

1815
import qualified Data.HashMap.Strict as HM
1916
import qualified Data.Text as T
@@ -29,7 +26,6 @@ import NeatInterpolation (trimming)
2926

3027
import Protolude
3128

32-
type RoleSettings = (HM.HashMap ByteString (HM.HashMap ByteString ByteString))
3329
type RoleIsolationLvl = HM.HashMap ByteString SQL.IsolationLevel
3430
type TimezoneNames = Set Text -- cache timezone names for prefer timezone=
3531

@@ -133,8 +129,8 @@ queryDbSettings preConfFunc =
133129
|]::Text
134130
decodeSettings = HD.rowList $ (,) <$> column HD.text <*> column HD.text
135131

136-
queryRoleSettings :: PgVersion -> Session (RoleSettings, RoleIsolationLvl)
137-
queryRoleSettings pgVer =
132+
queryRoleSettings :: Session RoleIsolationLvl
133+
queryRoleSettings =
138134
SQL.transactionNoRetry SQL.ReadCommitted SQL.Read $ SQL.statement mempty $ SQL.Statement sql HE.noParams (processRows <$> rows) True
139135
where
140136
sql = encodeUtf8 [trimming|
@@ -157,48 +153,20 @@ queryRoleSettings pgVer =
157153
FROM kv_settings
158154
WHERE key = 'default_transaction_isolation'
159155
)
160-
select
161-
kv.rolname,
162-
i.value as iso_lvl,
163-
coalesce(array_agg(row(kv.key, kv.value)) filter (where key <> 'default_transaction_isolation'), '{}') as role_settings
164-
from kv_settings kv
165-
join pg_settings ps on ps.name = kv.key and (ps.context = 'user' ${hasParameterPrivilege})
166-
left join iso_setting i on i.rolname = kv.rolname
167-
group by kv.rolname, i.value;
156+
select rolname, value
157+
from iso_setting;
168158
|]
169159

170-
hasParameterPrivilege
171-
| pgVer >= pgVersion150 = "or has_parameter_privilege(quote_ident(current_user)::regrole::oid, ps.name, 'set')"
172-
| otherwise = ""
173-
174-
processRows :: [(Text, Maybe Text, [(Text, Text)])] -> (RoleSettings, RoleIsolationLvl)
175-
processRows rs =
176-
let
177-
rowsWRoleSettings = [ (x, z) | (x, _, z) <- rs ]
178-
rowsWIsolation = [ (x, y) | (x, Just y, _) <- rs ]
179-
in
180-
( HM.fromList $ bimap encodeUtf8 (HM.fromList . ((encodeUtf8 *** encodeUtf8) <$>)) <$> rowsWRoleSettings
181-
, HM.fromList $ (encodeUtf8 *** toIsolationLevel) <$> rowsWIsolation
182-
)
160+
processRows :: [(Text, Text)] -> RoleIsolationLvl
161+
processRows =
162+
HM.fromList . fmap (bimap encodeUtf8 toIsolationLevel)
183163

184-
rows :: HD.Result [(Text, Maybe Text, [(Text, Text)])]
185-
rows = HD.rowList $ (,,) <$> column HD.text <*> nullableColumn HD.text <*> compositeArrayColumn ((,) <$> compositeField HD.text <*> compositeField HD.text)
164+
rows :: HD.Result [(Text, Text)]
165+
rows = HD.rowList $ (,) <$> column HD.text <*> column HD.text
186166

187167
column :: HD.Value a -> HD.Row a
188168
column = HD.column . HD.nonNullable
189169

190-
nullableColumn :: HD.Value a -> HD.Row (Maybe a)
191-
nullableColumn = HD.column . HD.nullable
192-
193-
compositeField :: HD.Value a -> HD.Composite a
194-
compositeField = HD.field . HD.nonNullable
195-
196-
compositeArrayColumn :: HD.Composite a -> HD.Row [a]
197-
compositeArrayColumn = arrayColumn . HD.composite
198-
199-
arrayColumn :: HD.Value a -> HD.Row [a]
200-
arrayColumn = column . HD.listArray . HD.nonNullable
201-
202170
param :: HE.Value a -> HE.Params a
203171
param = HE.param . HE.nonNullable
204172

‎src/PostgREST/Query/PreQuery.hs‎

Lines changed: 81 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
{-# LANGUAGE NamedFieldPuns #-}
2+
{-# LANGUAGE QuasiQuotes #-}
23
{-# LANGUAGE RecordWildCards #-}
4+
{-# LANGUAGE ExistentialQuantification #-}
35
{-|
46
Module : PostgREST.Query.PreQuery
57
Description : Builds queries that run prior to the main query
@@ -20,6 +22,7 @@ import qualified Hasql.Decoders as HD
2022
import qualified Hasql.DynamicStatements.Snippet as SQL hiding (sql)
2123
import qualified Hasql.Encoders as HE
2224
import qualified Hasql.Statement as SQL
25+
import NeatInterpolation (trimming)
2326
import qualified PostgreSQL.Binary.Encoding as PGBinary
2427
import Unsafe.Coerce (unsafeCoerce)
2528

@@ -39,12 +42,21 @@ import Protolude
3942
-- sets transaction variables
4043
txVarQuery :: DbActionPlan -> AppConfig -> AuthResult -> ApiRequest -> SQL.Statement () ()
4144
txVarQuery dbActPlan AppConfig{..} AuthResult{..} ApiRequest{..} =
42-
lmap (const settings) $ SQL.Statement txSettingsSql txSettingsParams HD.noResult configDbPreparedStatements
45+
if configDbConfig then
46+
lmap (const roleSettingsParams) $ SQL.Statement (txSettingsSqlWithRoleSettings configDbHasParameterPrivilege) txRoleSettingsParams HD.noResult configDbPreparedStatements
47+
else
48+
lmap (const settings) $ SQL.Statement txSettingsSqlWithoutRoleSettings txSettingsParams HD.noResult configDbPreparedStatements
4349
where
44-
settings = unzip $
45-
-- To ensure `GRANT SET ON PARAMETER <superuser_setting> TO authenticator` works, the role settings must be set before the impersonated role.
46-
-- Otherwise the GRANT SET would have to be applied to the impersonated role. See https://github.com/PostgREST/postgrest/issues/3045
47-
searchPathSetting : roleSettings ++
50+
settings =
51+
preRoleSettings ++ postRoleSettings
52+
53+
roleSettingsParams =
54+
(preRoleSettings, authRole, postRoleSettings)
55+
56+
preRoleSettings =
57+
[ searchPathSetting ]
58+
59+
postRoleSettings =
4860
[ roleSetting
4961
, claimsSetting
5062
, methodSetting
@@ -65,7 +77,6 @@ txVarQuery dbActPlan AppConfig{..} AuthResult{..} ApiRequest{..} =
6577
claims = authClaims & KM.insert "role" (JSON.String $ decodeUtf8 authRole) -- insert "role" to claims as well
6678

6779
roleSetting = ("role", authRole)
68-
roleSettings = HM.toList $ fromMaybe mempty $ HM.lookup authRole configRoleSettings
6980
appSettings = join bimap toUtf8 <$> configAppSettings
7081
timezoneSetting = maybe mempty (\(PreferTimezone tz) -> [("timezone", tz)]) $ preferTimezone iPreferences
7182
funcSettings = case dbActPlan of
@@ -75,15 +86,71 @@ txVarQuery dbActPlan AppConfig{..} AuthResult{..} ApiRequest{..} =
7586
let schemas = escapeIdentList (iSchema : configDbExtraSearchPath) in
7687
("search_path", schemas)
7788

78-
txSettingsSql :: ByteString
79-
txSettingsSql =
80-
"select set_config(setting, value, true) " <>
81-
"from unnest($1::text[], $2::text[]) with ordinality as _(setting, value, ordinality) " <>
82-
"order by ordinality"
83-
txSettingsParams :: HE.Params ([ByteString], [ByteString])
89+
txSettingsSqlWithoutRoleSettings :: ByteString
90+
txSettingsSqlWithoutRoleSettings =
91+
encodeUtf8 [trimming|
92+
select set_config(setting, value, true)
93+
from unnest($$1::text[], $$2::text[]) with ordinality as _(setting, value, ordinality)
94+
order by ordinality
95+
|]
96+
97+
txSettingsSqlWithRoleSettings :: Bool -> ByteString
98+
txSettingsSqlWithRoleSettings hasParameterPrivilege =
99+
encodeUtf8 [trimming|
100+
with pre_settings as (
101+
select 1::int as sort_order, ordinality, setting, value
102+
from unnest($$1::text[], $$2::text[]) with ordinality as _(setting, value, ordinality)
103+
),
104+
role_setting as (
105+
select unnest(r.rolconfig) as setting
106+
from pg_catalog.pg_roles r
107+
where r.rolname = $$3
108+
),
109+
kv_settings as (
110+
select setting_parts[1] as key, array_to_string(setting_parts[2:], '=') as value
111+
from role_setting
112+
cross join lateral string_to_array(setting, '=') as parsed(setting_parts)
113+
),
114+
role_settings as (
115+
select 2::int as sort_order, row_number() over (order by kv.key) as ordinality, kv.key as setting, kv.value
116+
from kv_settings kv
117+
join pg_catalog.pg_settings ps on ps.name = kv.key and ${parameterPrivilegeCheck}
118+
where kv.key <> 'default_transaction_isolation'
119+
),
120+
post_settings as (
121+
select 3::int as sort_order, ordinality, setting, value
122+
from unnest($$4::text[], $$5::text[]) with ordinality as _(setting, value, ordinality)
123+
),
124+
settings as (
125+
select * from pre_settings
126+
union all select * from role_settings
127+
union all select * from post_settings
128+
)
129+
select set_config(setting, value, true)
130+
from settings
131+
order by sort_order, ordinality
132+
|]
133+
where
134+
parameterPrivilegeCheck =
135+
if hasParameterPrivilege then
136+
[trimming|(ps.context = 'user' or has_parameter_privilege(quote_ident(current_user)::regrole::oid, ps.name, 'set'))|] :: Text
137+
else
138+
[trimming|(ps.context = 'user')|] :: Text
139+
140+
txSettingsParams :: HE.Params [(ByteString, ByteString)]
84141
txSettingsParams =
85-
(fst >$< txSettingsParameter) <>
86-
(snd >$< txSettingsParameter)
142+
unzip >$< ((fst >$< txSettingsParameter) <> (snd >$< txSettingsParameter))
143+
144+
txRoleSettingsParams :: HE.Params ([(ByteString, ByteString)], ByteString, [(ByteString, ByteString)])
145+
txRoleSettingsParams =
146+
((\(preSettings, _, _) -> preSettings) >$< txSettingsParams) <>
147+
((\(_, role, _) -> role) >$< txRoleParameter) <>
148+
((\(_, _, postSettings) -> postSettings) >$< txSettingsParams)
149+
150+
txRoleParameter :: HE.Params ByteString
151+
txRoleParameter =
152+
HE.param $ HE.nonNullable HE.unknown
153+
87154
txSettingsParameter :: HE.Params [ByteString]
88155
txSettingsParameter =
89156
HE.param $ HE.nonNullable txSettingsArray

‎test/io/test_io.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -957,9 +957,9 @@ def test_log_query(level, defaultenv):
957957
root_procs_regx = r".+: WITH base_types AS \(.+\) SELECT pn.nspname AS proc_schema, .+ FROM pg_proc p.+AND p.pronamespace = \$1::regnamespace"
958958
root_descr_regx = r".+: SELECT pg_catalog\.obj_description\(\$1::regnamespace, 'pg_namespace'\)"
959959
set_config_regx = (
960-
r".+: select set_config\(setting, value, true\) "
961-
r"from unnest\(\$1::text\[\], \$2::text\[\]\) with ordinality as _\(setting, value, ordinality\) "
962-
r"order by ordinality"
960+
r".+: with pre_settings as \(.+"
961+
r"select set_config\(setting, value, true\) "
962+
r"from settings order by sort_order, ordinality"
963963
)
964964

965965
output = drain_stdout(postgrest)

‎test/observability/ObsHelper.hs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ baseCfg = let secret = encodeUtf8 "reallyreallyreallyreallyverysafe" in
9090
, configDbRootSpec = Nothing
9191
, configDbSchemas = fromList ["test"]
9292
, configDbConfig = False
93+
, configDbHasParameterPrivilege = False
9394
, configDbPreConfig = Nothing
9495
, configDbTimezoneEnabled = True
9596
, configDbUri = "postgresql://"
@@ -117,7 +118,6 @@ baseCfg = let secret = encodeUtf8 "reallyreallyreallyreallyverysafe" in
117118
, configAdminServerPort = Nothing
118119
, configAdminServerUnixSocket = Nothing
119120
, configAdminServerUnixSocketMode = 432
120-
, configRoleSettings = mempty
121121
, configRoleIsoLvl = mempty
122122
, configInternalSCQuerySleepFst = Nothing
123123
, configInternalSCQuerySleepSnd = Nothing

‎test/spec/SpecHelper.hs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ baseCfg = let secret = encodeUtf8 "reallyreallyreallyreallyverysafe" in
131131
, configDbRootSpec = Nothing
132132
, configDbSchemas = fromList ["test"]
133133
, configDbConfig = False
134+
, configDbHasParameterPrivilege = False
134135
, configDbPreConfig = Nothing
135136
, configDbTimezoneEnabled = True
136137
, configDbUri = "postgresql://"
@@ -158,7 +159,6 @@ baseCfg = let secret = encodeUtf8 "reallyreallyreallyreallyverysafe" in
158159
, configAdminServerPort = Nothing
159160
, configAdminServerUnixSocket = Nothing
160161
, configAdminServerUnixSocketMode = 432
161-
, configRoleSettings = mempty
162162
, configRoleIsoLvl = mempty
163163
, configInternalSCQuerySleepFst = Nothing
164164
, configInternalSCQuerySleepSnd = Nothing

0 commit comments

Comments
 (0)