(refactor) Too easy to miss enabling a config as in-db and show it on the CLI

[steve-chavez]

Problem

This occurred on:

• server-timing-enabled missing on the CLI chore: add missing --example for server-timing-enabled #3069
• jwt-cache-max-lifetime missing in-db config on No in-db config for jwt-cache-max-lifetime #3100

This happens because the logic for the CLI and the in-db config is disconnected from the AppConfig type

postgrest/src/PostgREST/CLI.hs

Lines 124 to 233 in 0c65bf4

	exampleConfigFile :: [Char]
	exampleConfigFile =
	[str|## Admin server used for checks. It's disabled by default unless a port is specified.
	|# admin-server-port = 3001
	|
	|## The database role to use when no client authentication is provided
	|# db-anon-role = "anon"
	|
	|## Notification channel for reloading the schema cache
	|db-channel = "pgrst"
	|
	|## Enable or disable the notification channel
	|db-channel-enabled = true
	|
	|## Enable in-database configuration
	|db-config = true
	|
	|## Function for in-database configuration
	|## db-pre-config = "postgrest.pre_config"
	|
	|## Extra schemas to add to the search_path of every request
	|db-extra-search-path = "public"
	|
	|## Limit rows in response
	|# db-max-rows = 1000
	|
	|## Allow getting the EXPLAIN plan through the `Accept: application/vnd.pgrst.plan` header
	|# db-plan-enabled = false
	|
	|## Number of open connections in the pool
	|db-pool = 10
	|
	|## Time in seconds to wait to acquire a slot from the connection pool
	|# db-pool-acquisition-timeout = 10
	|
	|## Time in seconds after which to recycle pool connections
	|# db-pool-max-lifetime = 1800
	|
	|## Time in seconds after which to recycle unused pool connections
	|# db-pool-max-idletime = 30
	|
	|## Allow automatic database connection retrying
	|# db-pool-automatic-recovery = true
	|
	|## Stored proc to exec immediately after auth
	|# db-pre-request = "stored_proc_name"
	|
	|## Enable or disable prepared statements. disabling is only necessary when behind a connection pooler.
	|## When disabled, statements will be parametrized but won't be prepared.
	|db-prepared-statements = true
	|
	|## The name of which database schema to expose to REST clients
	|db-schemas = "public"
	|
	|## How to terminate database transactions
	|## Possible values are:
	|## commit (default)
	|## Transaction is always committed, this can not be overriden
	|## commit-allow-override
	|## Transaction is committed, but can be overriden with Prefer tx=rollback header
	|## rollback
	|## Transaction is always rolled back, this can not be overriden
	|## rollback-allow-override
	|## Transaction is rolled back, but can be overriden with Prefer tx=commit header
	|db-tx-end = "commit"
	|
	|## The standard connection URI format, documented at
	|## https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
	|db-uri = "postgresql://"
	|
	|# jwt-aud = "your_audience_claim"
	|
	|## Jspath to the role claim key
	|jwt-role-claim-key = ".role"
	|
	|## Choose a secret, JSON Web Key (or set) to enable JWT auth
	|## (use "@filename" to load from separate file)
	|# jwt-secret = "secret_with_at_least_32_characters"
	|jwt-secret-is-base64 = false
	|
	|## Enables and set JWT Cache max lifetime, disables caching with 0
	|# jwt-cache-max-lifetime = 0
	|
	|## Logging level, the admitted values are: crit, error, warn and info.
	|log-level = "error"
	|
	|## Determine if the OpenAPI output should follow or ignore role privileges or be disabled entirely.
	|## Admitted values: follow-privileges, ignore-privileges, disabled
	|openapi-mode = "follow-privileges"
	|
	|## Base url for the OpenAPI output
	|openapi-server-proxy-uri = ""
	|
	|## Configurable CORS origins
	|# server-cors-allowed-origins = ""
	|
	|server-host = "!4"
	|server-port = 3000
	|
	|## Allow getting the request-response timing information through the `Server-Timing` header
	|server-timing-enabled = false
	|
	|## Unix socket location
	|## if specified it takes precedence over server-port
	|# server-unix-socket = "/tmp/pgrst.sock"
	|
	|## Unix socket file mode
	|## When none is provided, 660 is applied by default
	|# server-unix-socket-mode = "660"
	|]

postgrest/src/PostgREST/Config/Database.hs

Lines 44 to 69 in 0c65bf4

	-- | In-db settings names
	dbSettingsNames :: [Text]
	dbSettingsNames =
	(prefix <>) <$>
	["db_aggregates_enabled"
	,"db_anon_role"
	,"db_pre_config"
	,"db_extra_search_path"
	,"db_max_rows"
	,"db_plan_enabled"
	,"db_pre_request"
	,"db_prepared_statements"
	,"db_root_spec"
	,"db_schemas"
	,"db_tx_end"
	,"jwt_aud"
	,"jwt_role_claim_key"
	,"jwt_secret"
	,"jwt_secret_is_base64"
	,"openapi_mode"
	,"openapi_security_active"
	,"openapi_server_proxy_uri"
	,"raw_media_types"
	,"server_trace_header"
	,"server_timing_enabled"
	]

postgrest/src/PostgREST/Config.hs

Lines 70 to 115 in 0c65bf4

	data AppConfig = AppConfig
	{ configAppSettings :: [(Text, Text)]
	, configDbAggregates :: Bool
	, configDbAnonRole :: Maybe BS.ByteString
	, configDbChannel :: Text
	, configDbChannelEnabled :: Bool
	, configDbExtraSearchPath :: [Text]
	, configDbMaxRows :: Maybe Integer
	, configDbPlanEnabled :: Bool
	, configDbPoolSize :: Int
	, configDbPoolAcquisitionTimeout :: Int
	, configDbPoolMaxLifetime :: Int
	, configDbPoolMaxIdletime :: Int
	, configDbPoolAutomaticRecovery :: Bool
	, configDbPreRequest :: Maybe QualifiedIdentifier
	, configDbPreparedStatements :: Bool
	, configDbRootSpec :: Maybe QualifiedIdentifier
	, configDbSchemas :: NonEmpty Text
	, configDbConfig :: Bool
	, configDbPreConfig :: Maybe QualifiedIdentifier
	, configDbTxAllowOverride :: Bool
	, configDbTxRollbackAll :: Bool
	, configDbUri :: Text
	, configFilePath :: Maybe FilePath
	, configJWKS :: Maybe JWKSet
	, configJwtAudience :: Maybe StringOrURI
	, configJwtRoleClaimKey :: JSPath
	, configJwtSecret :: Maybe BS.ByteString
	, configJwtSecretIsBase64 :: Bool
	, configJwtCacheMaxLifetime :: Int
	, configLogLevel :: LogLevel
	, configOpenApiMode :: OpenAPIMode
	, configOpenApiSecurityActive :: Bool
	, configOpenApiServerProxyUri :: Maybe Text
	, configServerCorsAllowedOrigins :: Maybe [Text]
	, configServerHost :: Text
	, configServerPort :: Int
	, configServerTraceHeader :: Maybe (CI.CI BS.ByteString)
	, configServerTimingEnabled :: Bool
	, configServerUnixSocket :: Maybe FilePath
	, configServerUnixSocketMode :: FileMode
	, configAdminServerPort :: Maybe Int
	, configRoleSettings :: RoleSettings
	, configRoleIsoLvl :: RoleIsolationLvl
	, configInternalSCSleep :: Maybe Int32
	}

Solution

Tie the AppConfig or other intermediate type to the CLI and the in-db logic.

[develop7]

The https://hackage.haskell.org/package/configuration-tools (source at https://github.com/alephcloud/hs-configuration-tools) could be the right tool for the job — it combines CLI options parser, *JSON instances, JSON/YAML config parsing (that's where it falls short for us, though — need to investigate how hard adding custom config formats' support could be), remote configs even, if needed.

The problem here is not every parameter is exposed to command line, configs and/or database, that's where it doesn't quite meet the needs as well.

[wolfgangwalther]

I was independently looking for alternatives to configurator-pg today and stumbled across configuration-tools as well. Looks promising. I would not mind a breaking change to switch from the current config format to a yaml based format. Maybe we could even provide a small migration script or something to load the old config and dump the new one.

One thing I like about configuration-tools is, that it looks like it combines a few very useful things. Some that we already do (config file, environment variables), but also new stuff on top (cli arguments, --help output with a information about the current build, dependencies, licences...). The one thing I am not sure about is how to integrate the in-database-configuration nicely... which is exactly part of this issue :)

[steve-chavez]

If we're going to change the file format, I think what would make most sense is using an INI format to be compatible with the pg service file (also see this blog post).

That way, we could support multiple databases (#2798) with a config like:

# pg_service.ini
[serviceone]
port=5432
user=stevechavez
dbname=stevechavez

[serviceone.postgrest]
jwt-secret = xxxx
db-pool = 10

[servicetwo]
port=5432
...

[servicetwo.postgrest]
jwt-secret = yyy
db-pool = 20
other-configs = ".."

This also has the advantage of being able to use psql like:

PGSERVICE=serviceone PGSERVICEFILE=pg_service.ini psql

psql (14.10 (Ubuntu 14.10-0ubuntu0.22.04.1))
Type "help" for help.

stevechavez=# 

[wolfgangwalther]

If we're going to change the file format, I think what would make most sense is using an INI format to be compatible with the pg service file (also see this blog post).

[...]

This also has the advantage of being able to use psql like:

Since this is part of libpq, this should already work with postgrest right now. Just export PGSERVICE before you start PostgREST.. should do it already. That also means it might be hard to use this file in a different way - i.e. use multiple services from that file at the same time, because that is not what libpq wants to do.

Supporting multiple databases would also easily be possible with yaml. The problem for this ini-based file format is, that it will again be impossible to represent anything more complex. YAML can do much more: arrays, objects, anchors, references, ... so much more.

[steve-chavez]

That also means it might be hard to use this file in a different way - i.e. use multiple services from that file at the same time, because that is not what libpq wants to do.

Right, so my intention was to process the ini file in PostgREST itself to use multiple dbs. And using the ini file we would still have the advantage of leveraging psql to test the different connections (PGSERVICE=anyservice PGSERVICEFILE=our_conf.ini psql). With YAML we cannot do that.

The problem for this ini-based file format is, that it will again be impossible to represent anything more complex. YAML can do much more: arrays, objects, anchors, references, ... so much more.

We don't have complex values now and we always need to consider that values have to be compatible with the in-db config. pgbouncer seems to do fine with an ini config too. So that doesn't seem like a blocker.