Add support for configuration via file/stdin

[jacobheun]

As the daemon grows our configuration flag options are likely to become cumbersome pretty quickly. I was talking with @bigs and @vasco-santos about needing to add support for specifying the multiaddrs the daemons libp2p node listens on. We could, and probably should, add a comma delimited string flag similar to how we add bootstrap addresses. Over time though, reading through all of the flags you have enabled for a node could get overwhelming.

I'd like to propose adding support for piping a config file to the daemon. For example:

$ cat config.json | p2p daemon

This would also have the nice benefit of being able to have node type configurations pre made for users; DHT booster, relay, rendezvous, etc.

If this sounds good, I can write up a spec for the json configuration.

[bigs]

👍 i think it should be as simple as trying to read stdin, and ignoring any command line options if data exists

[raulk]

See https://github.com/libp2p/go-libp2p-daemon/issues/35 for a discussion on configuration flags precisely!

[vyzo]

or we can simply pass the config file as argument, better than reading from stdin.

[jacobheun]

or we can simply pass the config file as argument, better than reading from stdin.

I'm good with this approach

[bigs]

yeah, fine by me. makes reproducibility easier too.

[bigs]

we should add, one of the motivating factors behind this was wanting to easily add lists (i.e. how we already do bootstrap nodes) and eventually add listen addresses to the configuration

[anacrolix]

If the configuration becomes too complex to represent with flags (😢 ), consider the merits of https://github.com/libp2p/go-libp2p/issues/526. If you're able to represent your configuration as a tree (explicit configuration structs with appropriate embedding), you can marshal the root of your configuration type to and from your configuration file format of choice (JSON/yaml etc.).

type Libp2pDaemonConfig struct {
    Host host.Config
    Dht dht.Config
    SomeMultiaddrsCozWhyNot []ma.Multiaddrs
}

Ultimately lists lack the structure to represent complex configuration. Maybe nip it in the bud before it gets out of hand?

[anacrolix]

It looks like @jacobheun already essentially suggested this, but https://github.com/libp2p/go-libp2p/issues/526 does add some context. It would be unfortunate to reexport a bunch of config in other packages in this way when they could be exposing it like that already.

[bigs]

interesting. the daemon ultimately has a different set of configuration options, but we are heavily leaning towards reading it from a JSON file.

[anacrolix]

@lgierth

[ghost]

or we can simply pass the config file as argument, better than reading from stdin.

But it'd also mean introducing a dependency on the filesystem. We'd make one-off invocations slightly more concise, but virtualization and automation more-than-slightly more involved.

But well I guess we can also have both ¯\_(ツ)_/¯ many unix tools accept a - filename meaning stdin.

As long as we make sure not to require stuff in the filesystem (apart from the control socket obviously, but that's not something the user needs to create themselves).

---

Another approach not mentioned here yet is to configure the daemon over its own API.

Disclaimer: I'm not yet very familiar with the daemon, or with all the teams that already have a need for the daemon (and it offering complex functionality). That need is there right now, so it's good to make small, swift, pragmatic steps. All I'd ask is we do steps that are in line with an overall longterm design goal and don't push ourselves into a corner that we eventually don't get out of. Things that trigger me: big config structs, god objects, long lists of cli flags, (bidirectionally) leaking abstractions between UIs and "business" code :P

There'd be a command running the daemon (let's say run), and a command starting and configuring it (let's say start). And I guess there would also be commands reloading/stopping/killing it. start would pass through run's stdout and stderr, so to the user it could look much the same as it does right now.

The handling of the user interface (CLI (and config file)) would be nicely confined to one command and decoupled from the actual work. We can focus mainly on API design instead. There's probably even a named design pattern for this.

Most importantly it'd help much on the way towards hot-reloading, which I think is super-desirable for anything networking-focused. Long way to get there, but very desirable, and as we get more networky down the roadmap, networking people will expect it.

It'll require pretty hefty design and refactoring work, but if we're going to refactor Host/Swarm in Go anyway... Don't need to have 100% hot-reloading ability right away, can do it step by step.

[raulk]

But it'd also mean introducing a dependency on the filesystem. We'd make one-off invocations slightly more concise, but virtualization and automation more-than-slightly more involved.

But well I guess we can also have both ¯\_(ツ)_/¯ many unix tools accept a - filename meaning stdin.

Exactly my thoughts. I'm not sure if - is the right interface here. I've seen it in vim - and others that expect a filename as an CLI arg, not as an option flag. If we want to do p2pd <config file> it should be ok, but I'm thinking we probably want to pass the config file as an option. So maybe -o <filename> for reading from a file, and -O for reading from stdin.

Anyway, it should be trivial to support both inputs, and agree it's the best path forward.

Most importantly it'd help much on the way towards hot-reloading, which I think is super-desirable for anything networking-focused. Long way to get there, but very desirable, and as we get more networky down the roadmap, networking people will expect it.

I'd shy away from watching the file and hot-reloading when it changes. It creates a significant security hole, and also it's not very polite. I'd propose reacting to SIGSTOP or another unused signal by re-loading the config file, or ignoring if we're reading from stdin.

[ghost]

So maybe -o <filename> for reading from a file, and -O for reading from stdin.

Yeah sounds good, or -f/-i (-o sounds like output)

I'd shy away from watching the file and hot-reloading when it changes. It creates a significant security hole, and also it's not very polite. I'd propose reacting to SIGSTOP or another unused signal by re-loading the config file, or ignoring if we're reading from stdin.

Agree on not watching the file and handling a signal instead, but SIGHUP or SIGUSR1 are most common for reloading. I've only ever seen SIGSTOP used for log rotation (in combination with SIGCONT), I think the process doesn't even get it (same as SIGKILL).

[Kubuxu]

SIGHUP is used for config reload for daemons (detached from a console) as a signal that the console closed doesn't make sense for daemons (go-ipfs is not a daemon in the traditional scheme). So if you are planning this I would go with SIGUSR1.

Also, as it is a config, maybe just go with --config flag.