Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use env interpolation in config #826

Merged
merged 6 commits into from
May 25, 2017
Merged

Use env interpolation in config #826

merged 6 commits into from
May 25, 2017

Conversation

begriffs
Copy link
Member

@begriffs begriffs commented Mar 6, 2017

Work in progress to use a combination of config file and environment variables in a Dockerfile.

@begriffs
Copy link
Member Author

begriffs commented Mar 6, 2017

Relates to the conversation in #808

@ruslantalpa
Copy link
Contributor

What a difference a good lib makes :)

@begriffs
Copy link
Member Author

begriffs commented Mar 6, 2017

There's still a problem. The docker/postgrest.conf uses environment variables for all settings, but our config library requires the env variables be present for interpolation or else it dies. So what's the problem, we set defaults in the Dockerfile with ENV, right? Well yes, but there is one big problem and one annoyance:

  • The annoyance is that we are specifying default values twice, like host and port. Once in the Dockerfile and once inside postgrest. A user can always specify the desired value so it's not really a problem, just inelegant.
  • The bigger problem. In most cases there is not a sensible default value for environment variables, so I assign them the empty string. However postgrest does not treat these as missing values, it treats them as the user's choice. The db-uri parameter for instance treats the empty string as an indication to use the default unix socket which will never work in our Docker container.

I propose we add logic to our configuration module that treats the empty string as a missing parameter. Interestingly this same problem of empty strings just arose in #822.

@begriffs
Copy link
Member Author

begriffs commented Mar 6, 2017

Hang on, looks like there is a successor to the configurator library: https://hackage.haskell.org/package/configurator-ng

Let's see what this one can do for us...

@deinspanjer
Copy link

I have a PR with new docker build stuff ready to test, but need an env aware config file. How is this testing coming?

@begriffs
Copy link
Member Author

I'll try to finish that up this weekend. Thanks for your patience with all this.

@begriffs
Copy link
Member Author

Note to self: once this branch is fixed and merged, check whether it helps #819.

@begriffs
Copy link
Member Author

@deinspanjer I ran into a problem and opened issue in the configurator-ng project about it. Hopefully they will have a suggestion.

COPY postgrest.conf /etc/postgrest.conf


ENV PGRST_DB_URI= \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o/ :heart:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this will work pretty well as a mix of env variables and mapping files into Docker if we can just get past the stumbling block of parsing quoted integers - lpsmith/configurator-ng#4

@ronalddddd
Copy link

Hope this is relevant. My current workaround to have an env vars-only docker image:

# https://github.com/ronalddddd/docker-postgrest/blob/master/entrypoint.sh
# ...
cat > /etc/postgrest.conf <<-EOF
db-uri = "postgres://${DBUSER}:${DBPASS}@${DBHOST}:${DBPORT}/${DBNAME}"
db-schema = "${DBSCHEMA}"
db-anon-role = "${ANONUSER}"
EOF

postgrest /etc/postgrest.conf

Docker image can be found here: https://hub.docker.com/r/ronalddddd/postgrest/

@begriffs
Copy link
Member Author

@ronalddddd, interesting, does it allow for all the options listed in the configuration section?

@begriffs
Copy link
Member Author

begriffs commented Apr 7, 2017

@roman I just read about Haskell-etc on the Haskell Cafe mailing list. It may be just what we're looking for in this pull request! Maybe you can advise.

The background is we read a simple config file on server startup. We would like to interpolate environment variables in the config file to allow it to be used in Docker without the need to map a config file into the container. I was trying the configurator-ng library, but it's unable to parse parse environment vars as integers, only as strings. Doesn't work with settings such as server-port which is an integer in our Haskell code.

@roman
Copy link

roman commented Apr 8, 2017

Hi @begriffs,

I guess the first thing is to define a spec file for your postgrest executable to use, in this spec file you would say which env vars align to your config keys. My first attempt would be something like this:

etc/entries:
  db-uri:
    etc/spec:
      env: PGRST_DB_URI

  db-schema:
    etc/spec:
      env: PGRST_DB_SCHEMA

  db-anon-role:
    etc/spec:
      env: PGRST_DB_ANON_ROLE

  db-pool:
    etc/spec:
      env: PGRST_DB_POOL


  server-host:
    etc/spec:
      env: PGRST_SERVER_HOST

  server-port:
    etc/spec:
      env: PGRST_SERVER_PORT


  server-proxy-url:
    etc/spec:
      env: PGRST_SERVER_PROXY_URL

  jwt-secret:
    etc/spec:
      env: PGRST_JWT_SECRET

  secret-is-base64:
    etc/spec:
      env: PGRST_SECRET_IS_BASE64

  max-rows:
    etc/spec:
      env: PGRST_MAX_ROWS

  pre-request:
    etc/spec:
      env: PGRST_PRE_REQUEST

And the Haskell side of things would look something like:

import Data.Aeson ((.:), (.:?))
import qualified Data.Aeson as JSON
import qualified System.Etc as Etc

parseConfiguration :: Etc.Config -> Maybe AppConfig
parseConfiguration config =
  let
    getValue key =
      Etc.getConfigValue [key] config
  in
    AppConfig
      <$> getValue "db-uri"
      <*> getValue "db-annon-value"
      <*> getValue "server-proxy-uri"
      <*> getValue "db-schema"
      <*> (fromMaybe "*4" <$> getValue "server-host")
      <*> (fromMaybe 3000 <$> getValue "server-port")
      <*> (fmap encodeUtf8 <$> getValue "jwt-secret")
      <*> (fromMaybe False <$> getValue "secret-is-base64")
      <*> (fromMaybe 10 <$> getValue "db-pool")
      <*> getValue "max-rows"
      <*> getValue "pre-request"
      <*> pure False

main :: IO ()
main = do
  spec <- Etc.readConfigSpec "spec.yaml"
  config <- Etc.resolveEnv spec
  parseConfiguration config

NOTE 1 you may also try using an Aeson.FromJSON instance for your AppConfig type, you'll need the AppConfig to be a sub-map in your config though.

NOTE 2 If dependency count is something you are mindful of, maybe having the configuration as JSON might be a better idea (sadly the yaml package adds many dependencies).

Let me know if you have more questions or if you want to pair to flesh things out, this API is still in testing and things might need some iterations to be fleshed out into a nicer interface.

Cheers, and thanks for considering etc.

@jesseadams
Copy link

This would be extremely useful.

@begriffs
Copy link
Member Author

@roman sorry I didn't acknowledge your careful explanation, thank you for researching that. I'm going to keep fighting with the existing library though because I want to continue supporting the simple key = val style config file for backward compatibility.

@ruslantalpa
Copy link
Contributor

Instead of fighting the lib to get env vars to work how about this.
The use of "env vars" in config is a "docker thing".
This can be easily solved with an entry point script even with the current version.
IMO, the only thing that needs change is the Dockerfile to use a entrypoint script like this.

#!/bin/bash
set -e


if [[ $SERVER_PROXY_URI != "" ]]; then SERVER_PROXY_URI="server-proxy-uri = \"${SERVER_PROXY_URI}\""; fi
if [[ $JWT_SECRET != "" ]]; then JWT_SECRET="jwt-secret = \"${JWT_SECRET}\""; fi
if [[ $MAX_ROWS != "" ]]; then MAX_ROWS="max-rows = ${MAX_ROWS}"; fi
if [[ $PRE_REQUEST != "" ]]; then PRE_REQUEST="pre-request = \"${PRE_REQUEST}\""; fi

CONF=/etc/postgrest.conf
touch $CONF
cat >> $CONF <<-EOS
db-uri = "$DB_URI"
db-schema = "$DB_SCHEMA"
db-anon-role = "$DB_ANON_ROLE"
db-pool = $DB_POOL

server-host = "$SERVER_HOST"
server-port = 3000

## base url for swagger output
# server-proxy-uri = ""
$SERVER_PROXY_URI

## choose a secret to enable JWT auth
## (use "@filename" to load from separate file)
# jwt-secret = "foo"
$JWT_SECRET
secret-is-base64 = $SECRET_IS_BASE64

## limit rows in response
# max-rows = 1000
$MAX_ROWS

## stored proc to exec immediately after auth
# pre-request = "stored_proc_name"
$PRE_REQUEST

EOS

exec postgrest /etc/postgrest.conf

@begriffs
Copy link
Member Author

That's an option for sure. I'm currently hacking on this PR a little this afternoon and let's just see if I can make a breakthrough.

@begriffs
Copy link
Member Author

YEAH! It seems to be working.


where
coerceInt :: (Read i, Integral i) => Value -> Maybe i
coerceInt (Number x) = rightToMaybe $ floatingOrInteger x
Copy link
Member Author

@begriffs begriffs May 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function works for Int or Integer, but Haskell throws a warning on this line because it defaults floatingOrInteger's left value to Double. I tried to cast it as Either Double i but GHC didn't catch on that I meant the same i as in the signature for coerceInt. Experimented with forall i. but didn't do the trick. Don't know how to clean that up.

@@ -1,3 +1,4 @@
{-# OPTIONS_GHC -fno-warn-type-defaults #-}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed this for the floatingOrInteger thing later on. Would rather not suppress a warning though.

@begriffs
Copy link
Member Author

Going to take the silence as a :shipit:

@begriffs begriffs merged commit 74227d3 into master May 25, 2017
@wolfgangwalther wolfgangwalther deleted the docker-with-env branch December 28, 2020 20:42
monacoremo pushed a commit to monacoremo/postgrest that referenced this pull request Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants