From af088cc9b50b3ac0d5c68759356e815fe05ca9d2 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Mon, 9 May 2022 21:00:00 -0300 Subject: [PATCH 1/2] PoC --- cmd/ipfs/daemon.go | 10 +- cmd/ipfs/init.go | 56 +++--- config/config.go | 177 ++++++++++++++++-- config/config_test.go | 53 ++++++ config/identity.go | 2 +- config/init.go | 53 ++++++ config/serialize/serialize.go | 8 +- config/serialize/serialize_test.go | 8 +- config/types.go | 4 +- plugin/loader/loader.go | 17 +- repo/common/common.go | 15 ++ repo/fsrepo/fsrepo.go | 88 +++++---- repo/fsrepo/fsrepo_test.go | 25 ++- .../migrations/ipfsfetcher/ipfsfetcher.go | 27 ++- repo/repo.go | 19 +- 15 files changed, 434 insertions(+), 128 deletions(-) diff --git a/cmd/ipfs/daemon.go b/cmd/ipfs/daemon.go index c1346059309..ab3a2a063d4 100644 --- a/cmd/ipfs/daemon.go +++ b/cmd/ipfs/daemon.go @@ -248,28 +248,26 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment if initialize && !fsrepo.IsInitialized(cctx.ConfigRoot) { cfgLocation, _ := req.Options[initConfigOptionKwd].(string) profiles, _ := req.Options[initProfileOptionKwd].(string) - var conf *config.Config + var conf config.UserConfigOverrides if cfgLocation != "" { if conf, err = cserial.Load(cfgLocation); err != nil { return err } - } - - if conf == nil { + } else { identity, err := config.CreateIdentity(os.Stdout, []options.KeyGenerateOption{ options.Key.Type(algorithmDefault), }) if err != nil { return err } - conf, err = config.InitWithIdentity(identity) + conf, err = config.NewUserConfigOverridesWithProfiles(identity, profiles) if err != nil { return err } } - if err = doInit(os.Stdout, cctx.ConfigRoot, false, profiles, conf); err != nil { + if err = doInit(os.Stdout, cctx.ConfigRoot, false, conf); err != nil { return err } } diff --git a/cmd/ipfs/init.go b/cmd/ipfs/init.go index dfbf01bb3a6..a61b7324073 100644 --- a/cmd/ipfs/init.go +++ b/cmd/ipfs/init.go @@ -2,21 +2,19 @@ package main import ( "context" - "encoding/json" "errors" "fmt" - "io" - "os" - "path/filepath" - "strings" - assets "github.com/ipfs/go-ipfs/assets" oldcmds "github.com/ipfs/go-ipfs/commands" core "github.com/ipfs/go-ipfs/core" "github.com/ipfs/go-ipfs/core/commands" + "github.com/ipfs/go-ipfs/repo/common" fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo" path "github.com/ipfs/go-path" unixfs "github.com/ipfs/go-unixfs" + "io" + "os" + "path/filepath" cmds "github.com/ipfs/go-ipfs-cmds" files "github.com/ipfs/go-ipfs-files" @@ -77,7 +75,7 @@ environment variable: algorithm, _ := req.Options[algorithmOptionName].(string) nBitsForKeypair, nBitsGiven := req.Options[bitsOptionName].(int) - var conf *config.Config + var conf config.UserConfigOverrides f := req.Files if f != nil { @@ -93,10 +91,11 @@ environment variable: return fmt.Errorf("expected a regular file") } - conf = &config.Config{} - if err := json.NewDecoder(file).Decode(conf); err != nil { + decoded, err := config.DecodeUserConfigOverrides(file) + if err != nil { return err } + conf = decoded } if conf == nil { @@ -115,36 +114,29 @@ environment variable: if err != nil { return err } - conf, err = config.InitWithIdentity(identity) + conf, err = config.NewUserConfigOverrides(identity) if err != nil { return err } } - profiles, _ := req.Options[profileOptionName].(string) - return doInit(os.Stdout, cctx.ConfigRoot, empty, profiles, conf) - }, -} - -func applyProfiles(conf *config.Config, profiles string) error { - if profiles == "" { - return nil - } - - for _, profile := range strings.Split(profiles, ",") { - transformer, ok := config.Profiles[profile] - if !ok { - return fmt.Errorf("invalid configuration profile: %s", profile) + if profiles, ok := req.Options[profileOptionName].(string); ok { + // Replace old config's profiles with the ones passed as arguments. + err := config.CheckProfiles(profiles) + if err != nil { + return err + } + err = common.MapSetKV(conf, "Profiles", profiles) + if err != nil { + return err + } } - if err := transformer.Transform(conf); err != nil { - return err - } - } - return nil + return doInit(os.Stdout, cctx.ConfigRoot, empty, conf) + }, } -func doInit(out io.Writer, repoRoot string, empty bool, confProfiles string, conf *config.Config) error { +func doInit(out io.Writer, repoRoot string, empty bool, conf config.UserConfigOverrides) error { if _, err := fmt.Fprintf(out, "initializing IPFS node at %s\n", repoRoot); err != nil { return err } @@ -157,10 +149,6 @@ func doInit(out io.Writer, repoRoot string, empty bool, confProfiles string, con return errRepoExists } - if err := applyProfiles(conf, confProfiles); err != nil { - return err - } - if err := fsrepo.Init(repoRoot, conf); err != nil { return err } diff --git a/config/config.go b/config/config.go index 1f4d9e83c87..ca4473bcb68 100644 --- a/config/config.go +++ b/config/config.go @@ -5,6 +5,8 @@ import ( "bytes" "encoding/json" "fmt" + "github.com/ipfs/go-ipfs/repo/common" + "io" "os" "path/filepath" "strings" @@ -12,9 +14,15 @@ import ( "github.com/mitchellh/go-homedir" ) -// Config is used to load ipfs config files. +// Config is the configuration used by an IPFS node. +// NOTE: It is a system-defined configuration with internal defaults that can be +// overridden by a user-defined JSON file (which is *not* the configuration itself). +// FIXME: Currently internal to the Repo interface, it should have its own place +// in the IpfsNode structure to decouple it from the user file. Similarly, +// everything here related to the user overrides should its own file. type Config struct { Identity Identity // local node's peer identity + Profiles string // profile from the user overrides to apply to defaults Datastore Datastore // local node's storage Addresses Addresses // local node's addresses Mounts Mounts // local node's mount points @@ -40,6 +48,11 @@ type Config struct { Internal Internal // experimental/unstable options } +// UserConfigOverrides is the Go map representing the loaded JSON file +// with user-defined configuration overrides. It is guaranteed on load +// that its structure matches a subset of the encoded Config struct. +type UserConfigOverrides map[string]interface{} + const ( // DefaultPathName is the default config dir name DefaultPathName = ".ipfs" @@ -111,22 +124,48 @@ func Marshal(value interface{}) ([]byte, error) { } func FromMap(v map[string]interface{}) (*Config, error) { - buf := new(bytes.Buffer) - if err := json.NewEncoder(buf).Encode(v); err != nil { + buf, err := JsonEncode(v) + if err != nil { return nil, err } - var conf Config - if err := json.NewDecoder(buf).Decode(&conf); err != nil { - return nil, fmt.Errorf("failure to decode config: %s", err) - } - return &conf, nil + return jsonDecodeConfig(buf) } func ToMap(conf *Config) (map[string]interface{}, error) { - buf := new(bytes.Buffer) - if err := json.NewEncoder(buf).Encode(conf); err != nil { + buf, err := JsonEncode(conf) + if err != nil { return nil, err } + return jsonDecodeMap(buf) +} + +// Clone copies the config. Use when updating. +// FIXME: This can't error. Refactor API. +func (c *Config) Clone() (*Config, error) { + buf, err := JsonEncode(c) + if err != nil { + return nil, err + } + return jsonDecodeConfig(buf) +} + +func JsonEncode(v interface{}) (*bytes.Buffer, error) { + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(v); err != nil { + return nil, fmt.Errorf("failure to encode config: %s", err) + } + return &buf, nil +} + +func jsonDecodeConfig(buf *bytes.Buffer) (*Config, error) { + var c Config + if err := json.NewDecoder(buf).Decode(&c); err != nil { + return nil, fmt.Errorf("failure to decode config: %s", err) + } + return &c, nil +} + +func jsonDecodeMap(buf *bytes.Buffer) (map[string]interface{}, error) { var m map[string]interface{} if err := json.NewDecoder(buf).Decode(&m); err != nil { return nil, fmt.Errorf("failure to decode config: %s", err) @@ -134,18 +173,120 @@ func ToMap(conf *Config) (map[string]interface{}, error) { return m, nil } -// Clone copies the config. Use when updating. -func (c *Config) Clone() (*Config, error) { - var newConfig Config +func NewUserConfigOverrides(identity Identity) (UserConfigOverrides, error) { + // FIXME: Is there an easier way to encode the Identity in a map? var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(identity); err != nil { + return nil, fmt.Errorf("failure to encode identity: %s", err) + } + var m map[string]interface{} + if err := json.NewDecoder(&buf).Decode(&m); err != nil { + return nil, fmt.Errorf("failure to decode identity: %s", err) + } - if err := json.NewEncoder(&buf).Encode(c); err != nil { - return nil, fmt.Errorf("failure to encode config: %s", err) + return map[string]interface{}{ + "Identity": m, + }, nil +} + +func NewUserConfigOverridesWithProfiles(identity Identity, profiles string) (UserConfigOverrides, error) { + overrides, err := NewUserConfigOverrides(identity) + if err != nil { + return nil, err } + err = CheckProfiles(profiles) + if err != nil { + return nil, err + } + overrides["Profiles"] = profiles + return overrides, nil +} - if err := json.NewDecoder(&buf).Decode(&newConfig); err != nil { - return nil, fmt.Errorf("failure to decode config: %s", err) +// OverrideMap replaces keys in left map with the right map, recursively traversing +// child maps until a non-map value is found. +// NOTE: Used for the JSON Config-to-map conversions: maps are expected to have +// same types, otherwise this will panic. +func OverrideMap(left, right map[string]interface{}) { + for key, rightVal := range right { + leftVal, found := left[key] + if !found { + // FIXME: For now nonexistent values in the left will be accepted + // and created from right. This is because JSON-decoded default config, left, + // still has a lot of `json:",omitempty"` that won't be present. In the future + // this should be removed. + left[key] = rightVal + continue + } + leftMap, ok := leftVal.(map[string]interface{}) + if !ok { + left[key] = rightVal + continue + } + if rightVal == nil { + return // FIXME: Do we want to clear config values? + // If override is empty we should error when loading the user override + // config file. + } + OverrideMap(leftMap, rightVal.(map[string]interface{})) + } +} + +// openConfig returns an error if the config file is not present. +func GetConfig(configFilePath string) (*Config, error) { + overrides, err := ReadUserConfigOverrides(configFilePath) + if err != nil { + return nil, err + } + var profiles string + p, err := common.MapGetKV(overrides, "Profiles") + if err == nil { + if profString, ok := p.(string); ok { + profiles = profString + } + } + + defaultConfig, err := DefaultConfig(profiles) + if err != nil { + return nil, err } - return &newConfig, nil + configMap, err := ToMap(defaultConfig) + if err != nil { + return nil, err + } + // This shoudln't be neccessary but just in case remove Identity, we'll never + // use a default one here. + delete(configMap, "Identity") + + OverrideMap(configMap, overrides) + config, err := FromMap(configMap) + if err != nil { + return nil, err + } + + return config, nil +} + +func ReadUserConfigOverrides(filename string) (UserConfigOverrides, error) { + f, err := os.Open(filename) + if err != nil { + //if os.IsNotExist(err) { + // err = ErrNotInitialized + //} + return nil, err + } + defer f.Close() + + return DecodeUserConfigOverrides(f) +} + +func DecodeUserConfigOverrides(r io.Reader) (UserConfigOverrides, error) { + var overrides UserConfigOverrides + dec := json.NewDecoder(r) + // FIXME: Check that this matches the contents of the Config struct. + dec.DisallowUnknownFields() + if err := dec.Decode(&overrides); err != nil { + return nil, fmt.Errorf("failure to decode user config: %s", err) + } + return overrides, nil } diff --git a/config/config_test.go b/config/config_test.go index dead06f8a23..43726059dc5 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -2,6 +2,9 @@ package config import ( "testing" + + "github.com/ipfs/go-ipfs/repo/common" + "github.com/stretchr/testify/require" ) func TestClone(t *testing.T) { @@ -27,3 +30,53 @@ func TestClone(t *testing.T) { t.Fatal("HTTP headers not preserved") } } + +// We are testing something that overrides *existing* values in the defaults. +// Anything other than a map is overridden as is, a map is inspected more deeply +// to override only set children. +func TestOverrideConfig(t *testing.T) { + overrideConfig := func(c *Config, m map[string]interface{}) *Config { + configMap, err := ToMap(c) + require.NoError(t, err) + OverrideMap(configMap, m) + overridden, err := FromMap(configMap) + require.NoError(t, err) + return overridden + } + cloneConfig := func(c *Config) *Config { + cloned, err := c.Clone() + require.NoError(t, err) + return cloned + } + MOD_TOKEN := "CONFIG-VALUE-MODIFIED-BY-TestOverrideMap" + modKey := func(c *Config, k string) *Config { + m, err := ToMap(c) + require.NoError(t, err) + require.NoError(t, common.MapSetKV(m, k, MOD_TOKEN)) + modConfig, err := FromMap(m) + require.NoError(t, err) + return modConfig + } + + testOverrides := func(c *Config) { + require.Equal(t, c, cloneConfig(c)) + require.Equal(t, c, overrideConfig(c, nil)) + require.Equal(t, modKey(c, "Identity.PeerID"), + overrideConfig(c, map[string]interface{}{ + "Identity": map[string]interface{}{ + "PeerID": MOD_TOKEN, + }, + })) + require.Equal(t, modKey(c, "Datastore.StorageMax"), + overrideConfig(c, map[string]interface{}{ + "Datastore": map[string]interface{}{ + "StorageMax": MOD_TOKEN, + }, + })) + } + + testOverrides(&Config{}) + defaultConfig, err := DefaultConfig("") + require.NoError(t, err) + testOverrides(defaultConfig) +} diff --git a/config/identity.go b/config/identity.go index f4e7c87200d..e7dba50319a 100644 --- a/config/identity.go +++ b/config/identity.go @@ -12,7 +12,7 @@ const PrivKeySelector = IdentityTag + "." + PrivKeyTag // Identity tracks the configuration of the local node's identity. type Identity struct { - PeerID string + PeerID string `json:",omitempty"` PrivKey string `json:",omitempty"` } diff --git a/config/init.go b/config/init.go index 8e54eaa5866..fffd55bb28f 100644 --- a/config/init.go +++ b/config/init.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "fmt" "io" + "strings" "time" "github.com/ipfs/interface-go-ipfs-core/options" @@ -21,6 +22,46 @@ func Init(out io.Writer, nBitsForKeypair int) (*Config, error) { return InitWithIdentity(identity) } +// FIXME: Join with InitWithIdentity. +func DefaultConfig(profiles string) (*Config, error) { + defaultConfig, err := InitWithIdentity(Identity{}) + if err != nil { + return nil, err + } + if err := applyProfiles(defaultConfig, profiles); err != nil { + return nil, err + } + return defaultConfig, nil +} + +func CheckProfiles(profiles string) error { + for _, profile := range strings.Split(profiles, ",") { + _, ok := Profiles[profile] + if !ok { + return fmt.Errorf("invalid configuration profile: %s", profile) + } + } + return nil +} + +func applyProfiles(conf *Config, profiles string) error { + if profiles == "" { + return nil + } + + if err := CheckProfiles(profiles); err != nil { + return err + } + + for _, profile := range strings.Split(profiles, ",") { + if err := Profiles[profile].Transform(conf); err != nil { + return err + } + } + return nil +} + +// FIXME: Rename to DefaultConfig. No identity. func InitWithIdentity(identity Identity) (*Config, error) { bootstrapPeers, err := DefaultBootstrapPeers() if err != nil { @@ -140,6 +181,18 @@ func DefaultDatastoreConfig() Datastore { } } +func DefaultDatastoreConfigMap() map[string]interface{} { + buf, err := JsonEncode(DefaultDatastoreConfig()) + if err != nil { + panic("failed to encode DefaultDatastoreConfig") + } + m, err := jsonDecodeMap(buf) + if err != nil { + panic("failed to decode DefaultDatastoreConfig") + } + return m +} + func badgerSpec() map[string]interface{} { return map[string]interface{}{ "type": "measure", diff --git a/config/serialize/serialize.go b/config/serialize/serialize.go index e51e9211575..2764a65c822 100644 --- a/config/serialize/serialize.go +++ b/config/serialize/serialize.go @@ -61,12 +61,16 @@ func encode(w io.Writer, value interface{}) error { } // Load reads given file and returns the read config, or error. -func Load(filename string) (*config.Config, error) { +func Load(filename string) (config.UserConfigOverrides, error) { var cfg config.Config err := ReadConfigFile(filename, &cfg) if err != nil { return nil, err } + cfgMap, err := config.ToMap(&cfg) + if err != nil { + return nil, err + } - return &cfg, err + return cfgMap, err } diff --git a/config/serialize/serialize_test.go b/config/serialize/serialize_test.go index 0c8e12f40c0..0be9e19a7e7 100644 --- a/config/serialize/serialize_test.go +++ b/config/serialize/serialize_test.go @@ -1,6 +1,7 @@ package fsrepo import ( + "github.com/ipfs/go-ipfs/repo/common" "os" "runtime" "testing" @@ -21,7 +22,12 @@ func TestConfig(t *testing.T) { if err != nil { t.Fatal(err) } - if cfgWritten.Identity.PeerID != cfgRead.Identity.PeerID { + peerId, err := common.MapGetKV(cfgRead, "Identity.PeerID") + if err != nil { + t.Fatal(err) + } + + if cfgWritten.Identity.PeerID != peerId.(string) { t.Fatal() } st, err := os.Stat(filename) diff --git a/config/types.go b/config/types.go index c33689c5b2a..cfbf263298c 100644 --- a/config/types.go +++ b/config/types.go @@ -21,7 +21,9 @@ func (o *Strings) UnmarshalJSON(data []byte) error { return err } if len(value) == 0 { - *o = []string{} + *o = nil + // FIXME: Unmarshal to nil instead of an instantiated (emtpy) `[]string{}` + // value to conform to the rest of the default (Go library) unmarshal logic. } else { *o = []string{value} } diff --git a/plugin/loader/loader.go b/plugin/loader/loader.go index 3c52a4105ad..4b8dab4f9a0 100644 --- a/plugin/loader/loader.go +++ b/plugin/loader/loader.go @@ -9,7 +9,6 @@ import ( "strings" config "github.com/ipfs/go-ipfs/config" - cserialize "github.com/ipfs/go-ipfs/config/serialize" "github.com/ipld/go-ipld-prime/multicodec" "github.com/ipfs/go-ipfs/core" @@ -97,14 +96,18 @@ type PluginLoader struct { func NewPluginLoader(repo string) (*PluginLoader, error) { loader := &PluginLoader{plugins: make(map[string]plugin.Plugin, len(preloadPlugins)), repo: repo} if repo != "" { - cfg, err := cserialize.Load(filepath.Join(repo, config.DefaultConfigFile)) - switch err { - case cserialize.ErrNotInitialized: - case nil: - loader.config = cfg.Plugins - default: + cfgFilename := filepath.Join(repo, config.DefaultConfigFile) + _, err := os.Open(cfgFilename) + if err != nil && !os.IsNotExist(err) { return nil, err } + if !os.IsNotExist(err) { + cfg, err := config.GetConfig(cfgFilename) + if err != nil { + return nil, err + } + loader.config = cfg.Plugins + } } for _, v := range preloadPlugins { if err := loader.Load(v); err != nil { diff --git a/repo/common/common.go b/repo/common/common.go index e9c56e65ee3..8d68a398f8d 100644 --- a/repo/common/common.go +++ b/repo/common/common.go @@ -61,6 +61,21 @@ func MapSetKV(v map[string]interface{}, key string, value interface{}) error { return nil } +type KeyValue struct { + Key string + Value interface{} +} + +func MapSetManyKV(v map[string]interface{}, kvs []KeyValue) error { + for _, kv := range kvs { + err := MapSetKV(v, kv.Key, kv.Value) + if err != nil { + return err + } + } + return nil +} + // Merges the right map into the left map, recursively traversing child maps // until a non-map value is found func MapMergeDeep(left, right map[string]interface{}) map[string]interface{} { diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index c35d5458dc7..18a55a0d5d2 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -96,16 +96,20 @@ type FSRepo struct { closed bool // path is the file-system path path string - // Path to the configuration file that may or may not be inside the FSRepo + // Path to the user configuration override file that may or may not be inside the FSRepo // path (see config.Filename for more details). + // FIXME: Rename to userConfigOverrideFilePath. configFilePath string // lockfile is the file system lock to prevent others from opening // the same fsrepo path concurrently lockfile io.Closer - config *config.Config - ds repo.Datastore - keystore keystore.Keystore - filemgr *filestore.FileManager + // System configuration that will be used by the running node. + // It is the merge of the internal defaultConfig and the external JSON file + // with the user overrides. + config *config.Config + ds repo.Datastore + keystore keystore.Keystore + filemgr *filestore.FileManager } var _ repo.Repo = (*FSRepo)(nil) @@ -236,7 +240,7 @@ func configIsInitialized(path string) bool { return true } -func initConfig(path string, conf *config.Config) error { +func initConfig(path string, conf config.UserConfigOverrides) error { if configIsInitialized(path) { return nil } @@ -275,7 +279,7 @@ func initSpec(path string, conf map[string]interface{}) error { // Init initializes a new FSRepo at the given path with the provided config. // TODO add support for custom datastores. -func Init(repoPath string, conf *config.Config) error { +func Init(repoPath string, conf config.UserConfigOverrides) error { // packageLock must be held to ensure that the repo is not initialized more // than once. @@ -290,7 +294,18 @@ func Init(repoPath string, conf *config.Config) error { return err } - if err := initSpec(repoPath, conf.Datastore.Spec); err != nil { + var profiles string + profilesFromMap, err := common.MapGetKV(conf, "Profiles") + if err != nil { + if pStr, ok := profilesFromMap.(string); ok { // FIXME: Why do we need this second check? + profiles = pStr + } + } + c, err := config.DefaultConfig(profiles) + if err != nil { + return err + } + if err := initSpec(repoPath, c.Datastore.Spec); err != nil { return err } @@ -390,11 +405,11 @@ func (r *FSRepo) SetAPIAddr(addr ma.Multiaddr) error { // openConfig returns an error if the config file is not present. func (r *FSRepo) openConfig() error { - conf, err := serialize.Load(r.configFilePath) + config, err := config.GetConfig(r.configFilePath) if err != nil { return err } - r.config = conf + r.config = config return nil } @@ -491,7 +506,8 @@ func (r *FSRepo) Close() error { return r.lockfile.Close() } -// Config the current config. This function DOES NOT copy the config. The caller +// Config returns the running system configuration. +// This function DOES NOT copy the config. The caller // MUST NOT modify it without first calling `Clone`. // // Result when not Open is undefined. The method may panic if it pleases. @@ -570,30 +586,32 @@ func (r *FSRepo) SetConfig(updated *config.Config) error { if err := serialize.WriteConfigFile(r.configFilePath, mergedMap); err != nil { return err } - // Do not use `*r.config = ...`. This will modify the *shared* config - // returned by `r.Config`. - r.config = updated - return nil + + return r.openConfig() } // GetConfigKey retrieves only the value of a particular key. func (r *FSRepo) GetConfigKey(key string) (interface{}, error) { - packageLock.Lock() - defer packageLock.Unlock() - - if r.closed { - return nil, errors.New("repo is closed") + c, err := r.Config() + if err != nil { + return nil, err } - var cfg map[string]interface{} - if err := serialize.ReadConfigFile(r.configFilePath, &cfg); err != nil { + configMap, err := config.ToMap(c) + if err != nil { return nil, err } - return common.MapGetKV(cfg, key) + + return common.MapGetKV(configMap, key) } // SetConfigKey writes the value of a particular key. func (r *FSRepo) SetConfigKey(key string, value interface{}) error { + if key == config.PrivKeySelector { + return fmt.Errorf("identity key %s cannot be replaced through the API", + config.PrivKeySelector) + } + packageLock.Lock() defer packageLock.Unlock() @@ -607,37 +625,27 @@ func (r *FSRepo) SetConfigKey(key string, value interface{}) error { return err } - // Load private key to guard against it being overwritten. - // NOTE: this is a temporary measure to secure this field until we move - // keys out of the config file. - pkval, err := common.MapGetKV(mapconf, config.PrivKeySelector) - if err != nil { - return err - } - // Set the key in the map. if err := common.MapSetKV(mapconf, key, value); err != nil { return err } - // replace private key, in case it was overwritten. - if err := common.MapSetKV(mapconf, config.PrivKeySelector, pkval); err != nil { - return err - } - // This step doubles as to validate the map against the struct // before serialization - conf, err := config.FromMap(mapconf) + buf, err := config.JsonEncode(mapconf) if err != nil { - return err + return fmt.Errorf("invalid user config file: %s", err) + } + _, err = config.DecodeUserConfigOverrides(buf) + if err != nil { + return fmt.Errorf("invalid user config file: %s", err) } - r.config = conf if err := serialize.WriteConfigFile(r.configFilePath, mapconf); err != nil { return err } - return nil + return r.openConfig() } // Datastore returns a repo-owned datastore. If FSRepo is Closed, return value diff --git a/repo/fsrepo/fsrepo_test.go b/repo/fsrepo/fsrepo_test.go index cf9aeabec0e..e4e6918995e 100644 --- a/repo/fsrepo/fsrepo_test.go +++ b/repo/fsrepo/fsrepo_test.go @@ -3,6 +3,7 @@ package fsrepo import ( "bytes" "context" + "github.com/ipfs/go-ipfs/repo/common" "io/ioutil" "os" "path/filepath" @@ -23,11 +24,23 @@ func testRepoPath(p string, t *testing.T) string { return name } +func emptyDatastore() config.UserConfigOverrides { + overrides, err := config.NewUserConfigOverrides(config.Identity{}) + if err != nil { + panic("failed to encode empty identity") + } + err = common.MapSetKV(overrides, "Datastore", config.DefaultDatastoreConfigMap()) + if err != nil { + panic("failed to set Datastore") + } + return overrides +} + func TestInitIdempotence(t *testing.T) { t.Parallel() path := testRepoPath("", t) for i := 0; i < 10; i++ { - assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "multiple calls to init should succeed") + assert.Nil(Init(path, emptyDatastore()), t, "multiple calls to init should succeed") } } @@ -42,8 +55,8 @@ func TestCanManageReposIndependently(t *testing.T) { pathB := testRepoPath("b", t) t.Log("initialize two repos") - assert.Nil(Init(pathA, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "a", "should initialize successfully") - assert.Nil(Init(pathB, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "b", "should initialize successfully") + assert.Nil(Init(pathA, emptyDatastore()), t, "a", "should initialize successfully") + assert.Nil(Init(pathB, emptyDatastore()), t, "b", "should initialize successfully") t.Log("ensure repos initialized") assert.True(IsInitialized(pathA), t, "a should be initialized") @@ -69,7 +82,7 @@ func TestDatastoreGetNotAllowedAfterClose(t *testing.T) { path := testRepoPath("test", t) assert.True(!IsInitialized(path), t, "should NOT be initialized") - assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t, "should initialize successfully") + assert.Nil(Init(path, emptyDatastore()), t, "should initialize successfully") r, err := Open(path) assert.Nil(err, t, "should open successfully") @@ -86,7 +99,7 @@ func TestDatastorePersistsFromRepoToRepo(t *testing.T) { t.Parallel() path := testRepoPath("test", t) - assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t) + assert.Nil(Init(path, emptyDatastore()), t) r1, err := Open(path) assert.Nil(err, t) @@ -106,7 +119,7 @@ func TestDatastorePersistsFromRepoToRepo(t *testing.T) { func TestOpenMoreThanOnceInSameProcess(t *testing.T) { t.Parallel() path := testRepoPath("", t) - assert.Nil(Init(path, &config.Config{Datastore: config.DefaultDatastoreConfig()}), t) + assert.Nil(Init(path, emptyDatastore()), t) r1, err := Open(path) assert.Nil(err, t, "first repo should open successfully") diff --git a/repo/fsrepo/migrations/ipfsfetcher/ipfsfetcher.go b/repo/fsrepo/migrations/ipfsfetcher/ipfsfetcher.go index 4a723293e6b..b7f3bba98aa 100644 --- a/repo/fsrepo/migrations/ipfsfetcher/ipfsfetcher.go +++ b/repo/fsrepo/migrations/ipfsfetcher/ipfsfetcher.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/ipfs/go-ipfs/repo/common" "io" "io/ioutil" "net/url" @@ -177,7 +178,7 @@ func initTempNode(ctx context.Context, bootstrap []string, peers []peer.AddrInfo if err != nil { return "", err } - cfg, err := config.InitWithIdentity(identity) + cfg, err := config.NewUserConfigOverrides(identity) if err != nil { return "", err } @@ -189,19 +190,27 @@ func initTempNode(ctx context.Context, bootstrap []string, peers []peer.AddrInfo } // configure the temporary node - cfg.Routing.Type = "dhtclient" - - // Disable listening for inbound connections - cfg.Addresses.Gateway = []string{} - cfg.Addresses.API = []string{} - cfg.Addresses.Swarm = []string{tempNodeTcpAddr} + err = common.MapSetManyKV(cfg, []common.KeyValue{ + {Key: "cfg.Routing.Type", Value: "dhtclient"}, + // Disable listening for inbound connections + {Key: "cfg.Addresses.Gateway", Value: []string{}}, + {Key: "cfg.Addresses.API", Value: []string{}}, + {Key: "cfg.Addresses.Swarm", Value: []string{tempNodeTcpAddr}}, + }) + if err != nil { + return "", err + } if len(bootstrap) != 0 { - cfg.Bootstrap = bootstrap + if err := common.MapSetKV(cfg, "cfg.Bootstrap", bootstrap); err != nil { + return "", err + } } if len(peers) != 0 { - cfg.Peering.Peers = peers + if err := common.MapSetKV(cfg, "cfg.Peering.Peers", peers); err != nil { + return "", err + } } // Assumes that repo plugins are already loaded diff --git a/repo/repo.go b/repo/repo.go index 00735eb94ee..ea6e7116c42 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -17,10 +17,17 @@ var ( ErrApiNotRunning = errors.New("api not running") ) +// FIXME: Anything returning config.Config here would now be returning +// the system config: internal system defaults with the UserConfigOverrides +// applied over them. // Repo represents all persistent data of a given ipfs node. type Repo interface { - // Config returns the ipfs configuration file from the repo. Changes made - // to the returned config are not automatically persisted. + // Config returns the running ipfs configuration from the system defaults + // overridden where applicable by a user-defined JSON file in the repo. + // Changes made to the returned config are not automatically persisted, but + // do impact on the running node. + // FIXME: Deprecate this in favor of GetSystemConfigKey to have a read-only + // configuration that is modified explicitly in SetSystemConfigKey. Config() (*config.Config, error) // BackupConfig creates a backup of the current configuration file using @@ -28,12 +35,18 @@ type Repo interface { BackupConfig(prefix string) (string, error) // SetConfig persists the given configuration struct to storage. + // FIXME: Deprecate this in favor of `SetConfigKey` to clearly + // expose which configuration options is being changed in the API call. SetConfig(*config.Config) error - // SetConfigKey sets the given key-value pair within the config and persists it to storage. + // SetConfigKey sets the given key-value pair within the system config and + // also persists it to the user configuration overrides file. SetConfigKey(key string, value interface{}) error // GetConfigKey reads the value for the given key from the configuration in storage. + // FIXME: Deprecate this and replace it with two distinct APIs: + // * GetSystemConfigKey: reads from the running system configuration. + // * GetUserConfigOverrideKey: reads from the user override file (`.ipfs/conf`). GetConfigKey(key string) (interface{}, error) // Datastore returns a reference to the configured data storage backend. From 28daa5e5aa876f81b4517d2dd994863a114dcb3b Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Tue, 31 May 2022 13:09:48 -0300 Subject: [PATCH 2/2] review GetConfigKey semantics --- core/commands/config.go | 2 + repo/fsrepo/fsrepo.go | 16 +++++++ test/sharness/t0021-config.sh | 85 ++++++++++++++++++----------------- 3 files changed, 62 insertions(+), 41 deletions(-) diff --git a/core/commands/config.go b/core/commands/config.go index 38e14c31da9..d3570e2ca64 100644 --- a/core/commands/config.go +++ b/core/commands/config.go @@ -507,6 +507,8 @@ func editConfig(filename string) error { return cmd.Run() } +// FIXME: Review expectations on this command. How will it work when we replace +// and "old" (full of defaults) config file with the "new" overrides? func replaceConfig(r repo.Repo, file io.Reader) error { var newCfg config.Config if err := json.NewDecoder(file).Decode(&newCfg); err != nil { diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index 18a55a0d5d2..6b9117cf273 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -592,6 +592,22 @@ func (r *FSRepo) SetConfig(updated *config.Config) error { // GetConfigKey retrieves only the value of a particular key. func (r *FSRepo) GetConfigKey(key string) (interface{}, error) { + // Since we're still allowing any field to be stored in the user file (even + // if it doesn't map to the Config struct) first look there, and only later + // (if not found) look in the running config. + // FIXME: Review if we can remove this expectation in the future. For now + // we only take this path if there are no errors (the config file might + // not even exist at the point of calling this API). + overrides, err := config.ReadUserConfigOverrides(r.configFilePath) + if err == nil { + value, err := common.MapGetKV(overrides, key) + if err == nil { + return value, nil + } + } + + // Not in the file, look in the running config (that includes the system + // default values). c, err := r.Config() if err != nil { return nil, err diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 5264908c73f..3a97e3ece4d 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -48,32 +48,34 @@ CONFIG_SET_JSON_TEST='{ } }' -test_profile_apply_revert() { - profile=$1 - inverse_profile=$2 - - test_expect_success "save expected config" ' - ipfs config show >expected - ' - - test_expect_success "'ipfs config profile apply ${profile}' works" ' - ipfs config profile apply '${profile}' - ' - - test_expect_success "profile ${profile} changed something" ' - ipfs config show >actual && - test_must_fail test_cmp expected actual - ' - - test_expect_success "'ipfs config profile apply ${inverse_profile}' works" ' - ipfs config profile apply '${inverse_profile}' - ' - - test_expect_success "config is back to previous state after ${inverse_profile} was applied" ' - ipfs config show >actual && - test_cmp expected actual - ' -} +# FIXME: Review profile tests. We no longer stored them in the config file except +# for the profile string itself. +#test_profile_apply_revert() { +# profile=$1 +# inverse_profile=$2 +# +# test_expect_success "save expected config" ' +# ipfs config show >expected +# ' +# +# test_expect_success "'ipfs config profile apply ${profile}' works" ' +# ipfs config profile apply '${profile}' +# ' +# +# test_expect_success "profile ${profile} changed something" ' +# ipfs config show >actual && +# test_must_fail test_cmp expected actual +# ' +# +# test_expect_success "'ipfs config profile apply ${inverse_profile}' works" ' +# ipfs config profile apply '${inverse_profile}' +# ' +# +# test_expect_success "config is back to previous state after ${inverse_profile} was applied" ' +# ipfs config show >actual && +# test_cmp expected actual +# ' +#} test_profile_apply_dry_run_not_alter() { profile=$1 @@ -117,21 +119,22 @@ test_config_cmd() { mv "$IPFS_PATH/config-moved" "$IPFS_PATH/config" ' - test_expect_success "setup for config replace test" ' - cp "$IPFS_PATH/config" newconfig.json && - sed -i"~" -e /PrivKey/d -e s/10GB/11GB/ newconfig.json && - sed -i"~" -e '"'"'/PeerID/ {'"'"' -e '"'"' s/,$// '"'"' -e '"'"' } '"'"' newconfig.json - ' - - test_expect_success "run 'ipfs config replace'" ' - ipfs config replace - < newconfig.json - ' - - test_expect_success "check resulting config after 'ipfs config replace'" ' - sed -e /PrivKey/d "$IPFS_PATH/config" > replconfig.json && - sed -i"~" -e '"'"'/PeerID/ {'"'"' -e '"'"' s/,$// '"'"' -e '"'"' } '"'"' replconfig.json && - test_cmp replconfig.json newconfig.json - ' +# FIXME: Review expectation of `config replace` command. +# test_expect_success "setup for config replace test" ' +# cp "$IPFS_PATH/config" newconfig.json && +# sed -i"~" -e /PrivKey/d -e s/10GB/11GB/ newconfig.json && +# sed -i"~" -e '"'"'/PeerID/ {'"'"' -e '"'"' s/,$// '"'"' -e '"'"' } '"'"' newconfig.json +# ' +# +# test_expect_success "run 'ipfs config replace'" ' +# ipfs config replace - < newconfig.json +# ' +# +# test_expect_success "check resulting config after 'ipfs config replace'" ' +# sed -e /PrivKey/d "$IPFS_PATH/config" > replconfig.json && +# sed -i"~" -e '"'"'/PeerID/ {'"'"' -e '"'"' s/,$// '"'"' -e '"'"' } '"'"' replconfig.json && +# test_cmp replconfig.json newconfig.json +# ' # SECURITY # Those tests are here to prevent exposing the PrivKey on the network