OxalisCommunity / oxalis

Oxalis - PEPPOL Access Point open source implementation - Core component
Other
124 stars 90 forks source link

ConfigModule.loadConfigurationFile should call `resolve()` to support substitutions #437

Closed darklajid closed 4 years ago

darklajid commented 4 years ago

The library used to handle the configuration file supports expansions, which both can be self-referencing (referencing another setting in the same file) and more importantly fall back to environment variables.

When running Oxalis in a container I'd like to use environment variables in the configuration and it seems that's literally just a single line / a single missing call as far as I can understand the code.

ConfigModule uses ConfigFactory.parseFile to read the oxalis.conf file - but the documentation for that call says¹

Parses a file into a Config instance. Does not call Config.resolve() ...

and the documentation for Config.resolve()² states

Returns a replacement config with all substitutions ...

So if I understand this correctly, adding a single .resolve() after ConfigFactory.parseFile in ConfigModule.loadConfigurationFile³ would support configuration values being supplied by the environment.

Example use case: The keystore pass. I want to have a config similar to

oxalis.keystore {
     path = theStoreFile.jks
     password = ${OXALIS_KEYSTORE_PASS}
     key.alias = theKeyAlias
     key.password = ${OXALIS_KEY_PASS}
 }

①: https://lightbend.github.io/config/latest/api/com/typesafe/config/ConfigFactory.html#parseFile-java.io.File-com.typesafe.config.ConfigParseOptions-

https://lightbend.github.io/config/latest/api/com/typesafe/config/Config.html#resolve--

https://github.com/difi/oxalis/blob/8dc2d6767db9b1b675cbd03adf413760e5e192cc/oxalis-commons/src/main/java/no/difi/oxalis/commons/config/ConfigModule.java#L59

klakegg commented 4 years ago

Thank you for spotting this. Could you please provide the pull request needed to include this functionality?

darklajid commented 4 years ago

As requested. It's literally tiny.

For some reason I wasn't able to run the full test suite, so I verified this minimal change by building a docker image, adding -DskipTests to the maven call in the Dockerfile.

Sanity:

Test change itself:

klakegg commented 4 years ago

Thank you for your contribution.

The updated edge Docker image (difi/oxalis:dev) is expected to be available soon.