ipfs / ipfs/kubo

Decouple the internal `Config` Go struct from the user config file in `.ipfs/conf`: use a subset of user overrides instead

Open
#8,925 2 comments 1 reaction 1 assignee Claimed by @schomatis View on GitHub
topic/config topic/technical debt
Dominant language
Go
Stars
17.1k
Forks
3.2k
Avg merge
3d 18h
Merged PRs (30d)
11

Description

_This issue is about fixing our internal technical debt without changing external UX. Some internal APIs will need to be broken but that can be minimized in a progressive deployment._

## Problem

We currently map the JSON data of the user configuration file (UCF) in `.ipfs/conf` to our internal [`Config` Go struct](https://github.com/ipfs/go-ipfs/blob/25cc85fa9359f907f348e0c2139f2b535313c56c/config/config.go#L16). This brings many problems:
* System defaults that might change over releases are hard-coded and locked in the UCF without the user realizing it (or even wanting to manage that particular configuration option).
* This causes the UCF to be bloated with default configuration options not set by the user, making it harder to realize which options have actually been changed or added. This makes the UCF harder to parse (both for the user and the developer triaging a new issue) with a giant blob of configuration options.
* There is no Go native way to mark a variable as unset. Variables always default to a Go value which might be different from the configuration default we may want that value to encode (the typical example is misinterpreting an unset limit with its default integer value of 0, instead of the expected "no limit" option). To avoid exposing the entire `Config` struct in the UCF we end up relying on stopgaps like the [`json:",omitempty"` option](https://pkg.go.dev/encoding/json#Marshal), but that ends up creating even more confusion as it only just hides the underlying coupling that remains between these two sources of configuration.

The last point in turn has given rise to "optional" variables, [`OptionalInteger` and related](https://github.com/ipfs/go-ipfs/blob/25cc85fa9359f907f348e0c2139f2b535313c56c/config/types.go#L268), to handle the JSON-GO mismatch of unspecified configuration options with the following drawbacks:
* Increased code bloat and associated maintaining costs, especially because we need one of this Go structures for each variable type the configuration encodes: `Flag`, `Strings`, `Priority`, `OptionalDuration`, `OptionalInteger`, `OptionalString`, and the list just keeps growing.
* Its `WithDefault()` API scatters system default values in the places that _access_ these values, instead of centralizing them in the `config` package where they are _defined_. These system defaults end up hard-coded deep in the code base, or we flat-out copy an external default structure field by field like:

https://github.com/ipfs/go-ipfs/blob/7162a63e9629258bac0b9d135acae4c3c1b9fb33/core/node/libp2p/relay.go#L28-L40

(Looking at the above list now, it seems like the copy/paste used to write the last 4 lines left the text `relayOpts.MaxReservations` untouched without adding the `PerPeer`, `PerIP`, and `PerASN` suffixes needed. This is a perfect example of the motivation for fixing technical debt like this one.)

## Proposed solution

We should clearly differentiate the two sources of configuration:

* **System configuration**: what the `go-ipfs` node is running with, encoded in a `Config` Go struct. It defaults to an internal set of values that depend on the node version.

* **User overrides**: Stored in `.ipfs/conf`, contains a JSON map of a _subset_ of the configuration options the user wants to impose over the system defaults.

**The user overrides are _not_ the JSON-serialized version of the system configuration.** We should not leak Go implementation details; for the user the only thing that exists is the JSON file (only internally do we use the `Config` struct to validate if those attributes are actually system-defined configuration options).

When the node starts it should load the `Config` with its internally-defined default values and only change the options specified in the user overrides, nothing else. The user overrides `.ipfs/conf` should be a bare-bones file after `ipfs init`.

All that is currently hard-coded in the `.ipfs/conf` file because of the exposure through the direct serialization of the `Config` struct, which at the moment is slowly and painfully being hidden through the cumbersome "optional" variables, will remain only in the `Config` struct as **system-defined defaults** (subject to change with the release version).

With this decoupling there is no more ambiguity on unspecified user options in the `.ipfs/conf` file since _all_ options will be set in the internal `Config` struct: an option the user doesn't specify just leaves in place the system default (no more `json:",omitempty"`).

## Implementation

The progressive road to implementation is illustrated in this [branch](https://github.com/ipfs/go-ipfs/compare/schomatis/fix/config/decouple-user-vs-system-config). The `Repo` interface is mostly maintained, what changes is that the `Config` struct (and related API calls) is no longer just the serialized user JSON file but instead the merge between the system defaults and JSON overrides (which are never serialized directly into the `Config` struct). The main objective of the first iteration is identifying which API consumers were relying on having all the configuration stored in the JSON file and start clearly differentiating between the user overrides and the internal system defaults: we should eliminate as much as possible the expectation of a *user configuration* (as it now exists) that contains all the configuration needed to run the node.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.