Feat/config envs docker

[gmasgras]

Implements #251

Allows passing ENVs to the Docker container to update go-ipfs configuration in /data/ipfs/config:
The ENVs should being with IPFS_CONFIG_ and are case sensitive. Nested properties are expressed via the _ separator:

"Swarm": {
   "AddrFilters": null,
   "ConnMgr": {
     "LowWater": 200,`
   }
 }

can be set via IPFS_CONFIG_Swarm_ConnMgr_LowWater=200

Multiple ENVs:

docker run -it \
-e IPFS_CONFIG_Swarm_ConnMgr_LowWater=200 \
-e IPFS_CONFIG_Swarm_ConnMgr_GracePeriod=49s \
-e IPFS_CONFIG_Gateway_Writable=false \
-e IPFS_CONFIG_Swarm_ConnMgr_Type=basic \
-e IPFS_CONFIG_Bootstrap="["/dnsaddr/bootstrap.libp2p.io/ipfs/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN","/dnsaddr/bootstrap.libp2p.io/ipfs/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u]"" \
go-ipfs:latest

[hannahhoward]

fix #251

[Stebalien]

This seems really brittle. Do we need to parse json manually like this or can we just ask the user to pass in valid JSON?

Alternatively, could we just skip manually updating the config file and use ipfs config --json key value?

[gmasgras]

There isn't a good/easy way to pass valid (correctly quoted) JSON through ENVs. We would have to manually parse the value of each ENV and add quotes to it when it's a JSON string (not a boolean or numeric). jq takes care of that.

I considered first using ipfs config but that would add extra parsing/checks when passing in booleans (needs--bool flag) or arrays (needs--json flag)

[Stebalien]

MYVAR='{"foobar": "baz"}'

?

---

My concern here isn't really jq, it's the fact that this code will break if it runs into any commas, square brackets, or newlines in a json array. And I'm pretty sure I'm missing a bunch of edge cases/bugs there.

[gmasgras]

That's not how we'd set foobar with this script. It will look more like IPFS_CONFIG_foobar=baz.
Array values are handled differently in lines 18-21

[Stebalien]

Array values are handled differently in lines 18-21

That's the buggy code I'm talking about. That's fine for a one-off script but not nearly robust enough for code we have to maintain.

That's not how we'd set foobar with this script. It will look more like IPFS_CONFIG_foobar=baz.

So we're just guessing the type? That'll fail for fields like Reprovider.Interval (which is a string/duration but can be a number in the config).

[Stebalien]

I'm not sure why you believe this is buggy, it assumes that anything starting with [ is an array. I think that's a reasonable assumption.

It:

• Deletes all brackets commas, regardless of where they show up.
• Will turn something like [/foo, /bar] into ["/foo", "", "/bar"].
• Doesn't support arrays of anything other than strings.

No, that doesn't actually fail:

That's exactly what I mean by fail. Reprovider.Interval must be a string but there's no way to specify a string with a value like "true", "0", "{}", etc. with this system.

---

So, instead of half-parsing json with some bash, let's just use json and keep this simple. We could also go with a typed approach, something like IPFS_CONFIG_STRING_foo_bar=... but it's probably not worth it.

[Stebalien]

This is absolutely not meant to be a generic ENV to JSON converter. (JSON is not even meant to be a config file format for the very reason that it shouldn't be necessary to need to know the type of a configuration value)

This is something we have to maintain. If we implement this, users will use it and report bugs in it. I don't want to maintain a bash script of hacks on hacks as we try to paper over all the edge cases here.

[Stebalien]

To be clear, I think we should totally support this. I just want it to be robust. That's why I'm suggesting we just go with something like:

docker run -it \
-e IPFS_CONFIG_Swarm_ConnMgr_LowWater='200' \
-e IPFS_CONFIG_Swarm_ConnMgr_GracePeriod='"49s"' \
-e IPFS_CONFIG_Gateway_Writable='false' \
-e IPFS_CONFIG_Swarm_ConnMgr_Type='"basic"' \
-e IPFS_CONFIG_Bootstrap='"["/dnsaddr/bootstrap.libp2p.io/ipfs/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN","/dnsaddr/bootstrap.libp2p.io/ipfs/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u"]"' \
go-ipfs:latest

[gmasgras]

there's no way to specify a string with a value like "true", "0", "{}", etc. with this system.

Where in the config is that needed or even possible (since it has to be valid JSON)? You are calling this code "hacks on hacks" because it doesn't cover some unrealistic use-case.. ?

The motivation for using jq to properly format the JSON values was to follow the robustness principle of "be conservative in what you do, be liberal in what you accept from others".

I can (reluctantly, since it makes the user's life harder) remove that line and force the users to deal with passing in properly formatted JSON if it moves this PR forward.

[Stebalien]

Where in the config is that needed or even possible (since it has to be valid JSON)?

This isn't what we need now but what we may need later. I don't want to box ourselves into a corner. That's why I said this is unmaintainable.

Also note, Reprovider.Interval is a string so we already need "0".

You are calling this code "hacks on hacks" because it doesn't cover some unrealistic use-case.. ?

I'm saying the code we'll end up with will be hacks on hacks as we try to cover all the edge cases as they come up. This code isn't yet hacks on hacks, it's just buggy.

---

We may also be able to rely on the type checking in go-ipfs. That is, we can run ipfs config --json "$thing" "$value" || ipfs config "$thing" "$value". That will let go-ipfs try treating the value as both a json value and a string. The config is type-checked internally so that shouldn't cause any issues.