lpsmith / configurator-ng

A Haskell library supporting flexible, dynamic file-based configuration.
Other
16 stars 7 forks source link

Using environment variables for integer values #4

Open begriffs opened 7 years ago

begriffs commented 7 years ago

My configuration file contains a field for a web server port. I'd like to read the value from an environment variable, but doing so always treats it as a string.

server-port = "$(FOO)"

Running with FOO exported to 3000 causes this error:

ConfigError {
  configErrorLocation = Key "" "server-port",
  configConversionError = Just [
    ConversionError {
      conversionErrorLoc = "boundedIntegerValue",
      conversionErrorWhy = TypeError,
      conversionErrorVal = Just (String "3000"),
      conversionErrorType = Just Int,
      conversionErrorMsg = Nothing
    }
  ]
}

I tried this too

server-port = $(FOO)

but it reports ParseError "test.conf" "endOfInput".

Is there something I can do in my code to allow both literal integers and those from environment variables?

(Context: https://github.com/begriffs/postgrest/pull/826)

begriffs commented 7 years ago

@lpsmith any thoughts?

lpsmith commented 7 years ago

Oh, sorry for taking so long to get back to you. So just a quick response for now:

Overall, I like the idea of allowing interpolation of value types other than strings, though there are a few small issues to consider. Nothing I foresee as any big deal though.

One possible workaround, is that you can use the Functor instance to allow strings or numbers in your parser, basically by allowing a string value that also happens to be a number. Or you can write your own ValueParser that does this. Though, I'm probably missing some things in the public API to write this sort of ValueParser nicely...

begriffs commented 7 years ago

I found a workaround:

coerceInt :: (Read i, Integral i) => Value -> Maybe i
coerceInt (Number x) = rightToMaybe $ floatingOrInteger x
coerceInt (String x) = readMaybe $ toS x
coerceInt _ = Nothing

Then I can do fromMaybe 3000 . join . fmap coerceInt <$> C.key "server-port"

lpsmith commented 7 years ago

Yeah, that's pretty close to one of the workarounds I had in mind, though perhaps done a little bit nicer. One downside is that this particular workaround will just default to port 3000 if it's not a number or a string that's parseable as an integer. The configurator-ng philosophy is that, if not an error, this should at least result in a warning that hey, I couldn't understand what you wrote, so I picked 3000 instead.

You could write your own ConfigParser instead of carrying out the computation in pure space, and thus fix the error message. I'm not sure if this can be done at the present time without delving into the internals, but if not, the API definitely needs fixed.

Either way, this requires a workaround in the source code, whereas from an operational perspective it might be nice for a system admin to be able to do this without having the programmers anticipate this particular use case.

So I think what would be acceptable is to be able to write server-port = $(FOO) and have this result in a boolean or a number depending on the syntax. Since the syntaxes supported by configurator-ng are disjoint, I don't see this as being a problem. I certainly don't want to allow "barewords" if the syntax isn't a boolean or a number; perhaps interpolating $(FOO) as a string would be acceptable in this case if there are quotes in the contents of FOO, but bare strings could create compatibility issues down the road. Lists are interesting thing to think about; I could definitely see it as useful to be able to interpolate $(PATH) into a list, but also seems like a bit much to bite off at the present time.

Also, I'm a bit annoyed that configurator chose $(...) for interpolation; the proper bash/perl syntax would be ${...}. The $(...) syntax would be shell interpolation in bash. Shell interpolation is an interesting thing to think about, but also way too much to deal with on this issue, as it offers a huge increase in power but also opens up a huge can of worms from a security perspective. (Although, if an attacker has control of your config files, you are probably already in a world of hurt.) And, I'd kind of like to add support for ${...} and deprecate $(...).

So I think the proper resolution to this issue is twofold:

  1. Ensure that one can write a ConfigParser with the public API that supports this kind of use case with nice error messages. And if not, fix the API so that you can.

  2. Add support for the server-port = $(FOO) syntax (perhaps with ${...} instead) that would allow booleans and numbers, but not (bare?) strings.

begriffs commented 7 years ago

Thanks for your consideration. If you do extend settings like server-port = ${FOO} to accommodate booleans and numbers let me know and I'll upgrade to the latest version and remove the coerceInt workaround. In fact you could make ${FOO} work with booleans and numbers while $(FOO) could be the backward compatible style which doesn't. Then no old code would break, but you could officially deprecate the old way for a while until some later major version removes support for it. Just an idea.

lpsmith commented 7 years ago

Yeah, after reviewing the current string syntax, unfortunately there doesn't seem to be a very nice migration path for strings; the conservative approach would be to introduce a new string syntax denoted by prefixing a character or something, e.g. s"...". The less conservative approach would be to break things and offer a tool to automatically migrate configuration files, which shouldn't be too difficult to implement.

In any case, I definitely will add environment interpolation for bools and integers eventually.