Untangling Configuration Var Mess

[jbenet]

We have three ways to express configuration variables to ipfs:

• $IPFS_DIR/config file
• ENV vars
• cli options / args

This is both good and bad. Good because all of these methods have specific use cases that make them much more preferrable to others (e.g. env vars in deployments, cli options are faster to change while experimenting than a config file, etc). Bad because it can create lots of confusion as to where each of these options should be defined in the code, and which methods it should support.

Here are some thoughts. Not sure what the right trajectory is yet.

1. Config: below the "ipfs core" barrier (i.e. all code within an ipfs node), we should use the Config exclusively (we have no access to options, AND should never use os.GetEnv). This means that all config variables the node can choose from have to be set on the config. (i think so far we've adhered to this).
2. Options: in commands, and commands exclusively, we parse options and call functions as need be. (this is already the case because only commands have access to options).
3. ENV vars for normal execution: we define all env vars for normal execution in one place cmd/env.go, read from the env, and set them on map on the cmdInvocation, or the execution context. Then, we could either force Config overrides to happen (i.e. env vars can only override config values) or Commands may access the env variables from the execution context. BUT, note os.GetEnv is disallowed (forcing users of env vars to add them to the one place we define them all, so we have them all listed in one file).
4. ENV vars for tests: for simplicity, one small exception for test case env vars: we are free to use, for example, os.GetEnv("NO_FUSE") from tests.

[btc]

What's the order of precedence?

CLI arg > ENV > config?

[whyrusleeping]

i think i like @maybebtc's order of precedence.

[jbenet]

that order of precedence seems right. Let's go with that until we find a strong case to deviate.

[rht]

ENV vars should be removed.

They are not tied to a specific daemon, hence not loggable (also who knows who/what set them and if they affect child processes). ENV vars for initial bootstrap are fine, but if further enabled they could be abused for persistent config in a bashrc. Also, extra overhead because people have to remember config precedence.

Besides ipfs config already provides the same convenience.

Reminds me of a mess in geant4 (a particle simulator framework) installation before they switched to cmake nowadays (https://aur.archlinux.org/packages/ge/geant4/PKGBUILD, https://github.com/Homebrew/homebrew-science/blob/master/geant.rb).

[jbenet]

ENV vars are used very often for all sorts of things. unfortunately it's the easiest way to setup deployments in many kinds of deployment systems (things like heroku and so on). I dont think we should remove them.

They are not tied to a specific daemon, hence not loggable (also who knows who/what set them and if they affect child processes). ENV vars for initial bootstrap are fine, but if further enabled they could be abused for persistent config in a bashrc. Also, extra overhead because people have to remember config precedence.

Agreed with all this, but there are cases where it makes more sense to use ENV vars, or mutating a file on disk is not a simple endeavor, and so using env vars as part of a command that gets run is the easiest thing. (it's not feasible to put everything as cli args).

ssh user@host IPFS_PATH=/foo/bar/ipfs ipfs daemon --init
ssh user@host IPFS_PATH=/foo/bar/ipfs ipfs pin add -r <path>

[daviddias]

Issue related -> #251

[eingenito]

If #6325 is merged this issue can be closed as we will have decided on a direction.

[Stebalien]

That PR makes it possible to configure a go-ipfs container using env variables but doesn't apply to go-ipfs itself. Personally, I'd kind of like to use environment variables for debugging and only debugging (except for containers). Current list: https://github.com/ipfs/go-ipfs/blob/master/docs/environment-variables.md

[Caian]

Why not use initialization scripts for containers? I think it is a common practice for database containers:

https://hub.docker.com/_/postgres

If you would like to do additional initialization in an image derived from this one, add one or more *.sql, *.sql.gz, or *.sh scripts under /docker-entrypoint-initdb.d (creating the directory if necessary). After the entrypoint calls initdb to create the default postgres user and database, it will run any *.sql files, run any executable *.sh scripts, and source any non-executable *.sh scripts found in that directory to do further initialization before starting the service.

[lidel]

Related clarification from #8326 (comment):

If we ever add ipfs config override via env variable, it should be supported in go-ipfs natively (go-ipfs/docs/environment-variables.md), and work as "ad-hoc" ephemeral override that does not mutate the config file.