ion icon indicating copy to clipboard operation
ion copied to clipboard

Use of environment variables for configuration

Open louneskmt opened this issue 3 years ago • 5 comments

Hey!

From what I'm seeing, it is not possible to use environment variables to configure ion-core and ion-bitcoin, rather than the current JSON files. These last can be pretty restrictive on certain setups/systems, and environment variables tend to become the standard.

Do you plan to add support for this anytime soon? I would gladly contribute if help is wanted.

louneskmt avatar Mar 31 '21 17:03 louneskmt

Hi @louneskmt,

Currently the way to configure ION node is through the use of the environment variables below:

ION_BITCOIN_CONFIG_FILE_PATH
ION_BITCOIN_VERSIONING_CONFIG_FILE_PATH
ION_CORE_CONFIG_FILE_PATH
ION_CORE_VERSIONING_CONFIG_FILE_PATH

Since there are many configuration parameters, exposing each as an environment variable might be problematic themselves, but open to discussion/proposal for improvement to overcome restrictions you are facing.

thehenrytsai avatar Mar 31 '21 21:03 thehenrytsai

Hey @thehenrytsai!

Yeah, I know these ones, but they only indicates a path to the actual JSON files.

It's true there are many configuration parameters and I can understand it could be hard to expose all of them as an environment variable.

I think we can distinguish two types of parameters (e.g. for bitcoin config):

  • internal, unlikely to change (e.g. sidetreeTransactionFeeMarkupPercentage, sidetreeTransactionPrefix, transactionPollPeriodInSeconds, etc)
  • external, related to connection details, likely to change (e.g. bitcoinPeerUri, bitcoinRpcUsername, bitcoinRpcPassword, mongoDbConnectionString and port)

It seems important/relevant to me to expose these last (external), as they are more likely to change from one system to another, and be set dynamically.

We would have the following environment variables available (keeping their names):


Bitcoin Config

BITCOIN_DATA_DIRECTORY
BITCOIN_PEER_URI
BITCOIN_RPC_USERNAME
BITCOIN_RPC_PASSWORD
MONGO_DB_CONNECTION_STRING
PORT

Core Config

BLOCKCHAIN_SERVICE_URI
IPFS_HTTP_API_ENDPOINT_URI
MONGO_DB_CONNECTION_STRING
PORT

While other parameters still have to be set using the JSON config file.

Let me know what you think.

louneskmt avatar Mar 31 '21 22:03 louneskmt

Receptive to the idea, a few thoughts:

  1. Would like to know the actual restrictive cases where reference to JSON files is not viable (hate to add features no one needs).
  2. Need to disambiguate config values from Bitcoin and Core, so a prefix is probably desirable (e.g. ION_CORE_PORT and ION_BITCOIN_PORT.
  3. Would want the environment variable names deterministically map to the config param names seen in JSON configs, so changes to the sidetree repo as well as ION repo are both necessary.
  4. Would prefer to have a universal approach where all params in the config files are overridable via environment variables, so not needing to special casing params that are "unlikely" to change.

thehenrytsai avatar Mar 31 '21 23:03 thehenrytsai

  1. A good reference is an Umbrel app. Umbrel is based on Docker containers working together, with several compose files and host scripts. Umbrel provides environment variables for almost everything, you can find an exhaustive list here. As you can see (for example) for the Mempool app, we populate app environment variables with ours directly. It works also with command flags, e.g. the Lightning Terminal app. Static configuration files for Umbrel apps can be pretty hard to handle.

  2. Sure.

  3. Yeah, we can just apply the naming convention MACRO_CASE, capitalising the parameter name, and delimiting words with an underscore. This way, we can easily map the param names to environment variables.

  4. Sorry, I haven't made this clear in my previous as it is generally the default behaviour for environment variables to overwrite config file params. Also, I proposed to distinguish the params in two types as you seemed to think that there were too many params ("exposing each as an environment variable might be problematic themselves"), but we can (and I think it is better too) expose each all of them.

louneskmt avatar Apr 01 '21 08:04 louneskmt

Sounds good. With above discussed, happy to review PR submitted to both sidetree and ION repo (both would be required)!

thehenrytsai avatar Apr 01 '21 23:04 thehenrytsai