correct NOTIFY config reload dying on error

This commit is contained in:
steve-chavez
2021-01-22 15:56:08 -05:00
committed by Steve Chavez
parent 17af56adb1
commit 6557f1f9c0
4 changed files with 68 additions and 39 deletions
+23 -16
View File
@@ -1,4 +1,5 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE MultiWayIf #-}
{-# LANGUAGE NamedFieldPuns #-}
@@ -66,7 +67,7 @@ main = do
CLI{cliCommand, cliPath} <- readCLIShowHelp env
-- build the 'AppConfig' from the config file path
conf <- readValidateConfig mempty env cliPath
conf <- either panic identity <$> readConfig mempty env cliPath
-- These are config values that can't be reloaded at runtime. Reloading some of them would imply restarting the web server.
let
@@ -103,10 +104,10 @@ main = do
-- Config that can change at runtime
refConf <- newIORef conf
let configRereader = reReadConfig pool gucConfigEnabled env cliPath refConf
let configRereader startingUp = reReadConfig startingUp pool gucConfigEnabled env cliPath refConf
-- re-read and override the config if db-load-guc-config is true
when gucConfigEnabled configRereader
when gucConfigEnabled $ configRereader True
case cliCommand of
CmdDumpConfig ->
@@ -148,13 +149,13 @@ main = do
-- Re-read the config on SIGUSR2
void $ installHandler sigUSR2 (
Catch $ configRereader >> putStrLn ("Config reloaded" :: Text)
Catch $ configRereader False
) Nothing
#endif
-- reload schema cache + config on NOTIFY
when dbChannelEnabled $
listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker configRereader
listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker $ configRereader False
-- ask for the OS time at most once per second
getTime <- mkAutoUpdate defaultUpdateSettings {updateAction = getCurrentTime}
@@ -310,7 +311,7 @@ loadSchemaCache pool actualPgVersion refConf refDbStructure = do
It uses the connectionWorker in case the LISTEN connection dies.
-}
listener :: ByteString -> Text -> P.Pool -> IORef AppConfig -> IORef (Maybe DbStructure) -> MVar ConnectionStatus -> IO () -> IO () -> IO ()
listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker configRereader = start
listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker configLoader = start
where
start = do
connStatus <- takeMVar mvarConnectionStatus -- takeMVar makes the thread wait if the MVar is empty(until there's a connection).
@@ -321,14 +322,13 @@ listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWo
Right db -> do
putStrLn $ "Listening for notifications on the " <> dbChannel <> " channel"
let channelToListen = N.toPgIdentifier dbChannel
cfLoader = configRereader >> putStrLn ("Config reloaded" :: Text)
scLoader = void $ loadSchemaCache pool actualPgVersion refConf refDbStructure -- It's not necessary to check the loadSchemaCache success here. If the connection drops, the thread will die and proceed to recover below.
N.listen db channelToListen
N.waitForNotifications (\_ msg ->
if | BS.null msg -> scLoader -- reload the schema cache
| msg == "reload schema" -> scLoader -- reload the schema cache
| msg == "reload config" -> cfLoader -- reload the config
| otherwise -> pure () -- Do nothing if anything else than an empty message is sent
if | BS.null msg -> scLoader -- reload the schema cache
| msg == "reload schema" -> scLoader -- reload the schema cache
| msg == "reload config" -> configLoader -- reload the config
| otherwise -> pure () -- Do nothing if anything else than an empty message is sent
) db
_ -> die errorMessage)
(\_ -> do -- if the thread dies, we try to recover
@@ -341,12 +341,19 @@ listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWo
retryMessage = "Retrying listening for notifications on the " <> dbChannel <> " channel.." :: Text
-- | Re-reads the config at runtime.
-- | If it panics(config path was changed, invalid setting), it'll show an error but won't kill the main thread.
reReadConfig :: P.Pool -> Bool -> Environment -> Maybe FilePath -> IORef AppConfig -> IO ()
reReadConfig pool gucConfigEnabled env path refConf = do
reReadConfig :: Bool -> P.Pool -> Bool -> Environment -> Maybe FilePath -> IORef AppConfig -> IO ()
reReadConfig startingUp pool gucConfigEnabled env path refConf = do
dbSettings <- if gucConfigEnabled then loadDbSettings pool else pure []
conf <- readValidateConfig dbSettings env path
atomicWriteIORef refConf conf
readConfig dbSettings env path >>= \case
Left err ->
if startingUp
then panic err -- die on invalid config if the program is starting up
else hPutStrLn stderr $ "Failed config reload. " <> err
Right conf -> do
atomicWriteIORef refConf conf
if startingUp
then pass
else putStrLn ("Config reloaded" :: Text)
-- | Dump DbStructure schema to JSON
dumpSchema :: P.Pool -> AppConfig -> IO LBS.ByteString
+16 -22
View File
@@ -30,7 +30,7 @@ module PostgREST.Config ( prettyVersion
, Environment
, readCLIShowHelp
, readEnvironment
, readValidateConfig
, readConfig
)
where
@@ -44,6 +44,7 @@ import Control.Lens (preview)
import Control.Monad (fail)
import Crypto.JWT (JWKSet, StringOrURI, stringOrUri)
import Data.Aeson (encode, toJSON)
import Data.Either.Combinators (mapLeft)
import Data.List (lookup)
import Data.List.NonEmpty (fromList, toList)
import Data.Maybe (fromJust)
@@ -52,7 +53,6 @@ import Data.Text (dropEnd, dropWhileEnd, filter,
intercalate, pack, replace, splitOn,
strip, stripPrefix, take, toLower,
toTitle, unpack)
import Data.Text.IO (hPutStrLn)
import Data.Version (versionBranch)
import Development.GitRev (gitHash)
import Numeric (readOct, showOct)
@@ -330,22 +330,18 @@ instance JustIfMaybe a (Maybe a) where
justIfMaybe a = Just a
-- | Parse the config file
readAppConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO AppConfig
readAppConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO (Either Text AppConfig)
readAppConfig dbSettings env optPath = do
-- Now read the actual config file
conf <- case optPath of
Just cfgPath -> catches (C.load cfgPath)
[ Handler (\(ex :: IOError) -> exitErr $ "Cannot open config file:\n\t" <> show ex)
, Handler (\(C.ParseError err) -> exitErr $ "Error parsing config file:\n" <> err)
Just cfgPath -> C.load cfgPath `catches`
[ Handler (\(ex :: IOError) -> panic $ "Cannot open config file: " <> show ex)
, Handler (\(C.ParseError err) -> panic $ "Error parsing config file: " <> err)
]
-- if no filename provided, start with an empty map to read config from environment
Nothing -> return M.empty
case C.runParser parseConfig conf of
Left err ->
exitErr $ "Error in config:\n\t" <> err
Right appConf ->
return appConf
pure $ mapLeft ("Error in config: " <>) $ C.runParser parseConfig conf
where
parseConfig =
@@ -395,7 +391,7 @@ readAppConfig dbSettings env optPath = do
parseSocketFileMode :: C.Key -> C.Parser C.Config FileMode
parseSocketFileMode k =
optString k >>= \case
Nothing -> pure $ 432 -- return default 660 mode if no value was provided
Nothing -> pure 432 -- return default 660 mode if no value was provided
Just fileModeText ->
case (readOct . unpack) fileModeText of
[] ->
@@ -518,16 +514,14 @@ readAppConfig dbSettings env optPath = do
splitOnCommas (C.String s) = strip <$> splitOn "," s
splitOnCommas _ = []
exitErr :: Text -> IO a
exitErr err = do
hPutStrLn stderr err
exitFailure
-- | Parse the AppConfig and validate it. Overrides the config options from env vars or db settings. Panics on invalid config options.
readValidateConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO AppConfig
readValidateConfig dbSettings env path = do
conf <- loadDbUriFile =<< loadSecretFile =<< readAppConfig dbSettings env path
return $ conf { configJWKS = parseSecret <$> configJwtSecret conf}
-- | Reads the config and overrides its parameters from files, env vars or db settings.
readConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO (Either Text AppConfig)
readConfig dbSettings env path =
readAppConfig dbSettings env path >>= \case
Left err -> pure $ Left err
Right appConf -> do
conf <- loadDbUriFile =<< loadSecretFile appConf
pure $ Right $ conf { configJWKS = parseSecret <$> configJwtSecret conf}
type Environment = M.Map [Char] Text
+12
View File
@@ -1955,3 +1955,15 @@ begin
perform pg_notify('pgrst', 'reload config');
perform pg_notify('pgrst', 'reload schema');
end $_$ volatile security definer language plpgsql ;
create or replace function test.invalid_role_claim_key_reload() returns void as $_$
begin
alter role postgrest_test_authenticator set pgrst."jwt-role-claim-key" = 'test';
perform pg_notify('pgrst', 'reload config');
end $_$ volatile security definer language plpgsql ;
create or replace function test.reset_invalid_role_claim_key() returns void as $_$
begin
alter role postgrest_test_authenticator set pgrst."jwt-role-claim-key" = '."a"."role"';
perform pg_notify('pgrst', 'reload config');
end $_$ volatile security definer language plpgsql ;
+17 -1
View File
@@ -154,7 +154,7 @@ def run(configpath=None, stdin=None, env=None, port=None):
if configpath:
command.append(configpath)
process = subprocess.Popen(command, stdin=subprocess.PIPE, env=env)
process = subprocess.Popen(command, stdin=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
try:
process.stdin.write(stdin or b"")
@@ -590,3 +590,19 @@ def test_max_rows_notify_reload(defaultenv):
# reset max-rows config on the db
postgrest.session.post("/rpc/reset_max_rows_config")
def invalid_role_claim_key_notify_reload(defaultenv):
"NOTIFY reload config should show an error if role-claim-key is invalid"
env = {
**defaultenv,
"PGRST_DB_LOAD_GUC_CONFIG": "true",
"PGRST_DB_CHANNEL_ENABLED": "true",
}
with run(env=env) as postgrest:
postgrest.session.post("/rpc/invalid_role_claim_key_reload")
assert "failed to parse role-claim-key value" in str(postgrest.process.stderr.readline())
postgrest.session.post("/rpc/reset_invalid_role_claim_key")