Config rework

[abukosek]

This is still in heavy WIP.

• Pass 1: Convert configuration cmdline flags into structs, make a global config struct, use that everywhere, add loading from YAML config file.
• Pass 2: Make the test runner and other non-node parts use the new config structs.
• Pass 3: Make everything work.

[kostko]

Instead of using a global config, can we pass it explicitly?

[abukosek]

Instead of using a global config, can we pass it explicitly?

Hmm, that seems like a lot of extra work. I'll first finish the current design and then we can discuss changing it.

A lot of modules need access to config options outside of their module, so we'd have to pass the entire global config struct to all modules anyway. And currently, only a few modules have some kind of init function where we could pass the config in explicitly, so all the others would have to be refactored to include something like that. I think the current global struct design is cleaner (load once, use anywhere).

[gw0]

Idea: In order to still support "re-usable" config files (after the removal of cmdline flags) it would be great to support some variable substitution before the config file gets parsed. Could be as trivial as replacing e.g. ${EXTERNAL_IP} or {{ EXTERNAL_IP }} with value of env variable EXTERNAL_IP or the value read from a .env file. Or a bit more powerful, like in docker-compose (https://docs.docker.com/compose/environment-variables/).

[abukosek]

Idea: In order to still support "re-usable" config files (after the removal of cmdline flags) it would be great to support some variable substitution before the config file gets parsed. Could be as trivial as replacing e.g. ${EXTERNAL_IP} or {{ EXTERNAL_IP }} with value of env variable EXTERNAL_IP or the value read from a .env file. Or a bit more powerful, like in docker-compose (https://docs.docker.com/compose/environment-variables/).

I don't think that the yaml package we're using to parse the config file supports this. This looks like something that should be done with external templating tools and not in oasis-core :)

[ptrus]

I think being able to set/override config values via environment variables should be possible, unless there is some hard reasoning not to support it.

One option would be using something like koanf (we use it in https://github.com/oasisprotocol/emerald-web3-gateway). It makes it really easy to use a Config struct (as defined in this PR) and automatically support config fields to be overridden via env variables, see example: https://github.com/oasisprotocol/emerald-web3-gateway/blob/a60756a8f1fb05cecd1d95bf31940a32812c9d8a/conf/config.go#L226-L255 (this is technically also possible to do with viper, but it has some issues like: spf13/viper#368 so I'd suggest avoiding it).

Or what @gw0 posted below.

[gw0]

I don't think that the yaml package we're using to parse the config file supports this. This looks like something that should be done with external templating tools and not in oasis-core :)

It does not, but setting env variables is convenient and adding support (https://github.com/a8m/envsubst) is as simple as:

import "github.com/a8m/envsubst"
data, err := envsubst.ReadFile("config.yaml")
err = yaml.Unmarshal(data, ...)

[abukosek]

Alright, I'll add that once I'm done with the test runner :)

EDIT: I like the envsubst variant better, so I'll use that.

[kostko]

This should also work if you set the global mode to validator?

[abukosek]

The previous code only checked the flag:
https://github.com/oasisprotocol/oasis-core/blob/master/go/oasis-node/cmd/common/flags/flags.go#L75-L77

[kostko]

Yeah this was wrong, we should make sure that if you set the global mode to validator your node will actually become a validator :-)

[kostko]

Should this go under the debug options?

[abukosek]

Hmm, it kind of makes sense to leave it here because it only sets the Tendermint log level to debug and isn't some unsupported debug flag.