From b7fc393e4922cdc3a7c0daa2b3e7d02e48719ab2 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 5 Dec 2020 22:04:15 +0100 Subject: [PATCH] feat: Read config directly from environment variables resolves #1624 --- main/Main.hs | 29 +++--- nix/docker/README.md | 12 +-- nix/docker/default.nix | 30 +----- nix/docker/postgrest.conf | 30 ------ nix/release/default.nix | 9 +- nix/release/docker-hub-full-description.md | 9 +- src/PostgREST/Config.hs | 112 +++++++++++++++------ test/io-tests/configs/no-defaults-env.yaml | 26 +++++ test/io-tests/test_io.py | 18 +++- 9 files changed, 151 insertions(+), 124 deletions(-) delete mode 100644 nix/docker/postgrest.conf create mode 100644 test/io-tests/configs/no-defaults-env.yaml diff --git a/main/Main.hs b/main/Main.hs index 3d69d7675..42f996aa3 100644 --- a/main/Main.hs +++ b/main/Main.hs @@ -33,8 +33,9 @@ import Text.Printf (hPrintf) import PostgREST.App (postgrest) import PostgREST.Config (AppConfig (..), CLI (..), Command (..), - configDbPoolTimeout', dumpAppConfig, - prettyVersion, readCLIShowHelp, + Environment, configDbPoolTimeout', + dumpAppConfig, prettyVersion, + readCLIShowHelp, readEnvironment, readValidateConfig) import PostgREST.DbStructure (getDbStructure, getPgVersion) import PostgREST.Error (PgError (PgError), checkIsFatal, @@ -63,11 +64,14 @@ main = do hSetBuffering stdin LineBuffering hSetBuffering stderr NoBuffering + -- read PGRST_ env variables + env <- readEnvironment + -- read path from commad line - opts <- readCLIShowHelp + opts <- readCLIShowHelp env -- build the 'AppConfig' from the config file path - conf <- readValidateConfig $ cliPath opts + conf <- readValidateConfig env $ cliPath opts -- These are config values that can't be reloaded at runtime. Reloading some of them would imply restarting the web server. let @@ -140,10 +144,11 @@ main = do Catch connWorker ) Nothing - -- Re-read the config on SIGUSR2 - void $ installHandler sigUSR2 ( - Catch $ reReadConfig (cliPath opts) refConf - ) Nothing + -- Re-read the config on SIGUSR2, but only if we have a config file + when (isJust $ cliPath opts) $ + void $ installHandler sigUSR2 ( + Catch $ reReadConfig env (cliPath opts) refConf + ) Nothing #endif -- reload schema cache on NOTIFY @@ -330,12 +335,12 @@ listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWo errorMessage = "Could not listen for notifications on the " <> dbChannel <> " channel" :: Text retryMessage = "Retrying listening for notifications on the " <> dbChannel <> " channel.." :: Text +#ifndef mingw32_HOST_OS -- | Re-reads the config at runtime. Invoked on SIGUSR2. -- | If it panics(config path was changed, invalid setting), it'll show an error but won't kill the main thread. -#ifndef mingw32_HOST_OS -reReadConfig :: FilePath -> IORef AppConfig -> IO () -reReadConfig path refConf = do - conf <- readValidateConfig path +reReadConfig :: Environment -> Maybe FilePath -> IORef AppConfig -> IO () +reReadConfig env path refConf = do + conf <- readValidateConfig env path atomicWriteIORef refConf conf putStrLn ("Config file reloaded" :: Text) #endif diff --git a/nix/docker/README.md b/nix/docker/README.md index 7a1d48276..eb8ee4238 100644 --- a/nix/docker/README.md +++ b/nix/docker/README.md @@ -2,8 +2,8 @@ In order to build an optimal PostgREST Docker image, we create the image from scratch (i.e., without a parent image like `debian` or `alpine`), and only -include the files that are essential for running PostgREST (the static -PostgREST binary and a `postgrest.conf`). +include the file that is essential for running PostgREST: the static +PostgREST binary. This is similar to what you would get with the following `Dockerfile`: @@ -17,12 +17,6 @@ FROM scratch # need to include for running the application. ADD /absolute/path/to/postgrest /bin/postgrest -# Include a default configuration file. -ADD /absolute/path/to/postgrest.conf /etc/postgrest.conf - -ENV PGRST_DB_URI= \ - ... - EXPOSE 3000 # This is the user id that Docker will run our image under by default. Note @@ -32,7 +26,7 @@ EXPOSE 3000 # can be run under any user you specify. USER 1000 -CMD [ "/bin/postgrest", "/etc/postgrest.conf" ] +CMD [ "/bin/postgrest" ] ``` # Building the Docker image with Nix diff --git a/nix/docker/default.nix b/nix/docker/default.nix index 09ed9e3c3..f88523292 100644 --- a/nix/docker/default.nix +++ b/nix/docker/default.nix @@ -4,9 +4,6 @@ , checkedShellScript }: let - config = - ./postgrest.conf; - image = dockerTools.buildImage { name = "postgrest"; @@ -19,34 +16,11 @@ let extraCommands = '' - mkdir etc - cp ${config} etc/postgrest.conf rmdir share ''; config = { - Cmd = [ "/bin/postgrest" "/etc/postgrest.conf" ]; - Env = [ - "PGRST_DB_URI=postgresql://?user=postgres" - "PGRST_DB_SCHEMA=public" - "PGRST_DB_ANON_ROLE=" - "PGRST_DB_POOL=100" - "PGRST_DB_POOL_TIMEOUT=10" - "PGRST_DB_EXTRA_SEARCH_PATH=public" - "PGRST_DB_CHANNEL=pgrst" - "PGRST_DB_CHANNEL_ENABLED=false" - "PGRST_SERVER_HOST=*4" - "PGRST_SERVER_PORT=3000" - "PGRST_OPENAPI_SERVER_PROXY_URI=" - "PGRST_JWT_SECRET=" - "PGRST_SECRET_IS_BASE64=false" - "PGRST_JWT_AUD=" - "PGRST_MAX_ROWS=" - "PGRST_PRE_REQUEST=" - "PGRST_ROLE_CLAIM_KEY=.role" - "PGRST_ROOT_SPEC=" - "PGRST_RAW_MEDIA_TYPES=" - ]; + Cmd = [ "/bin/postgrest" ]; User = "1000"; ExposedPorts = { "3000/tcp" = { }; @@ -65,4 +39,4 @@ buildEnv { name = "postgrest-docker"; paths = [ load.bin ]; - } // { inherit image config; } + } // { inherit image; } diff --git a/nix/docker/postgrest.conf b/nix/docker/postgrest.conf deleted file mode 100644 index 82f022dfc..000000000 --- a/nix/docker/postgrest.conf +++ /dev/null @@ -1,30 +0,0 @@ -# See https://postgrest.org/en/stable/configuration.html#configuration - -# Required settings - -db-uri = "$(PGRST_DB_URI)" -db-schema = "$(PGRST_DB_SCHEMA)" -db-anon-role = "$(PGRST_DB_ANON_ROLE)" - -# Optional settings - -db-pool = "$(PGRST_DB_POOL)" -db-pool-timeout = "$(PGRST_DB_POOL_TIMEOUT)" -db-extra-search-path = "$(PGRST_DB_EXTRA_SEARCH_PATH)" - -db-channel = "$(PGRST_DB_CHANNEL)" -db-channel-enabled = "$(PGRST_DB_CHANNEL_ENABLED)" - -server-host = "$(PGRST_SERVER_HOST)" -server-port = "$(PGRST_SERVER_PORT)" - -openapi-server-proxy-uri = "$(PGRST_OPENAPI_SERVER_PROXY_URI)" -jwt-secret = "$(PGRST_JWT_SECRET)" -secret-is-base64 = "$(PGRST_SECRET_IS_BASE64)" -jwt-aud = "$(PGRST_JWT_AUD)" -role-claim-key = "$(PGRST_ROLE_CLAIM_KEY)" - -max-rows = "$(PGRST_MAX_ROWS)" -pre-request = "$(PGRST_PRE_REQUEST)" -root-spec = "$(PGRST_ROOT_SPEC)" -raw-media-types = "$(PGRST_RAW_MEDIA_TYPES)" diff --git a/nix/release/default.nix b/nix/release/default.nix index 67f309e8a..9f9efc24a 100644 --- a/nix/release/default.nix +++ b/nix/release/default.nix @@ -94,17 +94,12 @@ let | ${jq}/bin/jq -r .token )" - # Plug the default config file into the full description. - defaultConfig="$(cat ${docker.config})" - export DEFAULT_CONFIG="$defaultConfig" - fullDescription="$(${envsubst}/bin/envsubst < ${fullDescription})" - - # Patch the full description. + # Patch both descriptions. responseCode="$( ${curl}/bin/curl -s --write-out "%{response_code}" \ --output /dev/null -H "Authorization: JWT $token" -X PATCH \ --data-urlencode description@${description} \ - --data-urlencode "full_description=$fullDescription" \ + --data-urlencode full_description@${fullDescription} \ "https://hub.docker.com/v2/repositories/$DOCKER_REPO/postgrest/" )" diff --git a/nix/release/docker-hub-full-description.md b/nix/release/docker-hub-full-description.md index 5a9964164..9f42096a1 100644 --- a/nix/release/docker-hub-full-description.md +++ b/nix/release/docker-hub-full-description.md @@ -15,13 +15,8 @@ write from scratch. To learn how to use this container, see the [PostgREST Docker documentation](https://postgrest.com/en/stable/install.html#docker). -You can configure the PostgREST image by setting the enviroment variables used -in the default `/etc/postgrest.conf` file or overriding that file. This is the -default configuration file: - -``` -$DEFAULT_CONFIG -``` +You can configure the PostgREST image by setting +[enviroment variables](https://postgrest.org/en/stable/configuration.html). # How this image is built diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index 9cd480a3d..d6f919103 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -12,9 +12,12 @@ turned in configurable behaviour if needed. Other hardcoded options such as the minimum version number also belong here. -} -{-# LANGUAGE LambdaCase #-} -{-# LANGUAGE ScopedTypeVariables #-} -{-# LANGUAGE TemplateHaskell #-} +{-# LANGUAGE FlexibleContexts #-} +{-# LANGUAGE FlexibleInstances #-} +{-# LANGUAGE LambdaCase #-} +{-# LANGUAGE MultiParamTypeClasses #-} +{-# LANGUAGE ScopedTypeVariables #-} +{-# LANGUAGE TemplateHaskell #-} {-# OPTIONS_GHC -fno-warn-type-defaults #-} module PostgREST.Config ( prettyVersion @@ -24,7 +27,9 @@ module PostgREST.Config ( prettyVersion , AppConfig (..) , configDbPoolTimeout' , dumpAppConfig + , Environment , readCLIShowHelp + , readEnvironment , readValidateConfig ) where @@ -33,6 +38,7 @@ import qualified Data.ByteString as B import qualified Data.ByteString.Base64 as B64 import qualified Data.ByteString.Char8 as BS import qualified Data.Configurator as C +import qualified Data.Map.Strict as M import qualified Text.PrettyPrint.ANSI.Leijen as L import Control.Lens (preview) @@ -41,6 +47,7 @@ import Crypto.JWT (JWKSet, StringOrURI, stringOrUri) import Data.Aeson (encode, toJSON) import Data.Either.Combinators (fromRight', whenLeft) import Data.List.NonEmpty (fromList, toList) +import Data.Maybe (fromJust) import Data.Scientific (floatingOrInteger) import Data.Text (dropEnd, dropWhileEnd, filter, intercalate, pack, replace, splitOn, @@ -51,6 +58,7 @@ import Data.Version (versionBranch) import Development.GitRev (gitHash) import Numeric (readOct, showOct) import Paths_postgrest (version) +import System.Environment (getEnvironment) import System.IO.Error (IOError) import System.Posix.Types (FileMode) @@ -74,7 +82,7 @@ import Protolude.Conv (toS) -- | Command line interface options data CLI = CLI { cliCommand :: Command - , cliPath :: FilePath } + , cliPath :: Maybe FilePath } data Command = CmdRun @@ -133,8 +141,8 @@ docsVersion :: Text docsVersion = "v" <> dropEnd 1 (dropWhileEnd (/= '.') prettyVersion) -- | Read command line interface options. Also prints help. -readCLIShowHelp :: IO CLI -readCLIShowHelp = customExecParser parserPrefs opts +readCLIShowHelp :: Environment -> IO CLI +readCLIShowHelp env = customExecParser parserPrefs opts where parserPrefs = prefs showHelpOnError @@ -164,10 +172,16 @@ readCLIShowHelp = customExecParser parserPrefs opts ) ) <*> - strArgument ( + optionalWithEnvironment (strArgument ( metavar "FILENAME" <> - help "Path to configuration file" - ) + help "Path to configuration file (optional with PGRST_ environment variables)" + )) + + optionalWithEnvironment :: Alternative f => f a -> f (Maybe a) + optionalWithEnvironment v + | M.null env = Just <$> v + | otherwise = optional v + exampleCfg :: Doc exampleCfg = vsep . map (text . toS) . lines $ @@ -300,25 +314,39 @@ dumpAppConfig conf = secret = fromMaybe mempty $ configJwtSecret c showSocketMode c = showOct (fromRight' $ configServerUnixSocketMode c) "" +-- This class is needed for the polymorphism of overrideFromEnvironment +-- because C.required and C.optional have different signatures +class JustIfMaybe a b where + justIfMaybe :: a -> b + +instance JustIfMaybe a a where + justIfMaybe a = a + +instance JustIfMaybe a (Maybe a) where + justIfMaybe a = Just a + -- | Parse the config file -readAppConfig :: FilePath -> IO AppConfig -readAppConfig cfgPath = do +readAppConfig :: Environment -> Maybe FilePath -> IO AppConfig +readAppConfig env optPath = do -- Now read the actual config file - conf <- 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) - ] + 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) + ] + -- 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 parsing config file:\n\t" <> err + exitErr $ "Error in config:\n\t" <> err Right appConf -> return appConf where parseConfig = AppConfig - <$> (fmap (fmap coerceText) <$> C.subassocs "app.settings" C.value) + <$> parseAppSettings "app.settings" <*> reqString "db-anon-role" <*> (fromMaybe "pgrst" <$> optString "db-channel") <*> (fromMaybe False <$> optBool "db-channel-enabled") @@ -353,9 +381,28 @@ readAppConfig cfgPath = do <*> (fmap unpack <$> optString "server-unix-socket") <*> parseSocketFileMode "server-unix-socket-mode" + parseAppSettings :: C.Key -> C.Parser C.Config [(Text, Text)] + parseAppSettings key = addFromEnv . fmap (fmap coerceText) <$> C.subassocs key C.value + where + addFromEnv f = M.toList $ M.union fromEnv $ M.fromList f + fromEnv = M.mapKeys fromJust $ M.filterWithKey (\k _ -> isJust k) $ M.mapKeys normalize env + normalize k = ("app.settings." <>) <$> stripPrefix "PGRST_APP_SETTINGS_" (toS k) + + overrideFromEnvironment :: JustIfMaybe a b => + (C.Key -> C.Parser C.Value a -> C.Parser C.Config b) -> + C.Key -> (C.Value -> a) -> C.Parser C.Config b + overrideFromEnvironment necessity key coercion = + case M.lookup name env of + Just envVal -> pure $ justIfMaybe $ coercion $ C.String envVal + Nothing -> necessity key (coercion <$> C.value) + where + name = "PGRST_" <> map capitalize (toS key) + capitalize '-' = '_' + capitalize c = toUpper c + parseSocketFileMode :: C.Key -> C.Parser C.Config (Either Text FileMode) parseSocketFileMode k = - C.optional k C.string >>= \case + overrideFromEnvironment C.optional k coerceText >>= \case Nothing -> pure $ Right 432 -- return default 660 mode if no value was provided Just fileModeText -> case (readOct . unpack) fileModeText of @@ -368,7 +415,7 @@ readAppConfig cfgPath = do parseJwtAudience :: C.Key -> C.Parser C.Config (Maybe StringOrURI) parseJwtAudience k = - C.optional k C.string >>= \case + overrideFromEnvironment C.optional k coerceText >>= \case Nothing -> pure Nothing -- no audience in config file Just aud -> case preview stringOrUri (unpack aud) of Nothing -> fail "Invalid Jwt audience. Check your configuration." @@ -377,7 +424,7 @@ readAppConfig cfgPath = do parseLogLevel :: C.Key -> C.Parser C.Config LogLevel parseLogLevel k = - C.optional k C.string >>= \case + overrideFromEnvironment C.optional k coerceText >>= \case Nothing -> pure LogError Just "" -> pure LogError Just "crit" -> pure LogCrit @@ -388,7 +435,7 @@ readAppConfig cfgPath = do parseTxEnd :: C.Key -> ((Bool, Bool) -> Bool) -> C.Parser C.Config Bool parseTxEnd k f = - C.optional k C.string >>= \case + overrideFromEnvironment C.optional k coerceText >>= \case -- RollbackAll AllowOverride Nothing -> pure $ f (False, False) Just "" -> pure $ f (False, False) @@ -414,19 +461,19 @@ readAppConfig cfgPath = do Nothing -> alias reqString :: C.Key -> C.Parser C.Config Text - reqString k = C.required k C.string + reqString k = overrideFromEnvironment C.required k coerceText optString :: C.Key -> C.Parser C.Config (Maybe Text) - optString k = mfilter (/= "") <$> C.optional k C.string + optString k = mfilter (/= "") <$> overrideFromEnvironment C.optional k coerceText optValue :: C.Key -> C.Parser C.Config (Maybe C.Value) - optValue k = C.optional k C.value + optValue k = overrideFromEnvironment C.optional k identity optInt :: (Read i, Integral i) => C.Key -> C.Parser C.Config (Maybe i) - optInt k = join <$> C.optional k (coerceInt <$> C.value) + optInt k = join <$> overrideFromEnvironment C.optional k coerceInt optBool :: C.Key -> C.Parser C.Config (Maybe Bool) - optBool k = join <$> C.optional k (coerceBool <$> C.value) + optBool k = join <$> overrideFromEnvironment C.optional k coerceBool coerceText :: C.Value -> Text coerceText (C.String s) = s @@ -461,9 +508,9 @@ readAppConfig cfgPath = do exitFailure -- | Parse the AppConfig and validate it. Panic on invalid config options. -readValidateConfig :: FilePath -> IO AppConfig -readValidateConfig path = do - conf <- loadDbUriFile =<< loadSecretFile =<< readAppConfig path +readValidateConfig :: Environment -> Maybe FilePath -> IO AppConfig +readValidateConfig env path = do + conf <- loadDbUriFile =<< loadSecretFile =<< readAppConfig env path -- Checks that the provided proxy uri is formated correctly when (isMalformedProxyUri $ toS <$> configOpenApiServerProxyUri conf) $ panic @@ -474,6 +521,13 @@ readValidateConfig path = do whenLeft (configServerUnixSocketMode conf) panic return $ conf { configJWKS = parseSecret <$> configJwtSecret conf} +type Environment = M.Map [Char] Text + +readEnvironment :: IO Environment +readEnvironment = getEnvironment <&> pgrst + where + pgrst env = M.filterWithKey (\k _ -> "PGRST_" `isPrefixOf` k) $ M.map pack $ M.fromList env + {-| The purpose of this function is to load the JWT secret from a file if configJwtSecret is actually a filepath and replaces some characters if the JWT diff --git a/test/io-tests/configs/no-defaults-env.yaml b/test/io-tests/configs/no-defaults-env.yaml new file mode 100644 index 000000000..e711131ba --- /dev/null +++ b/test/io-tests/configs/no-defaults-env.yaml @@ -0,0 +1,26 @@ +PGRST_APP_SETTINGS_test2: test +PGRST_APP_SETTINGS_test: test +PGRST_DB_ANON_ROLE: root +PGRST_DB_CHANNEL: postgrest +PGRST_DB_CHANNEL_ENABLED: true +PGRST_DB_EXTRA_SEARCH_PATH: public, test +PGRST_DB_MAX_ROWS: 1000 +PGRST_DB_POOL: 1 +PGRST_DB_POOL_TIMEOUT: 100 +PGRST_DB_PREPARED_STATEMENTS: false +PGRST_DB_PRE_REQUEST: please_run_fast +PGRST_DB_ROOT_SPEC: openapi_v3 +PGRST_DB_SCHEMAS: multi, tenant,setup +PGRST_DB_TX_END: rollback-allow-override +PGRST_DB_URI: tmp_db +PGRST_JWT_AUD: 'https://postgrest.org' +PGRST_JWT_ROLE_CLAIM_KEY: '.user[0]."real-role"' +PGRST_JWT_SECRET: c2VjdXJpdHl0aHJvdWdob2JzY3VyaXR5 +PGRST_JWT_SECRET_IS_BASE64: true +PGRST_LOG_LEVEL: info +PGRST_OPENAPI_SERVER_PROXY_URI: 'https://postgrest.org' +PGRST_RAW_MEDIA_TYPES: application/vnd.pgrst.config +PGRST_SERVER_HOST: 0.0.0.0 +PGRST_SERVER_PORT: 80 +PGRST_SERVER_UNIX_SOCKET: /tmp/pgrst_io_test.sock +PGRST_SERVER_UNIX_SOCKET_MODE: 777 diff --git a/test/io-tests/test_io.py b/test/io-tests/test_io.py index 4075513bf..997080941 100644 --- a/test/io-tests/test_io.py +++ b/test/io-tests/test_io.py @@ -58,10 +58,14 @@ def dburi(): return os.getenv("POSTGREST_TEST_CONNECTION").encode("utf-8") -def dumpconfig(configpath, moreenv=None, stdin=None): +def dumpconfig(configpath=None, moreenv=None, stdin=None): "Dump the config as parsed by PostgREST." + env = {**os.environ, **(moreenv or {})} - command = ["postgrest", "--dump-config", configpath] + command = ["postgrest", "--dump-config"] + if configpath: + command += [configpath] + process = subprocess.Popen( command, env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE ) @@ -143,6 +147,16 @@ def test_expected_config(expectedconfig): assert dumpconfig(CONFIGSDIR / expectedconfig.name) == expected +def test_expected_config_from_environment(): + "Config should be read directly from environment without config file." + + envfile = (CONFIGSDIR / "no-defaults-env.yaml").read_text() + env = {k: str(v) for k, v in yaml.load(envfile, Loader=yaml.Loader).items()} + + expected = (CONFIGSDIR / "expected" / "no-defaults.config").read_text() + assert dumpconfig(moreenv=env) == expected + + @pytest.mark.parametrize( "config", [conf for conf in CONFIGSDIR.iterdir() if conf.suffix == ".config"],