From c5649c2468488dc06c8ce94a96b947421527f95b Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 12:38:31 +0300 Subject: [PATCH 01/28] Fix JSON config validation --- config/config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 91fb219a2a1..0b75fe5cbc1 100644 --- a/config/config.go +++ b/config/config.go @@ -119,7 +119,9 @@ func FromMap(v map[string]interface{}) (*Config, error) { return nil, err } var conf Config - if err := json.NewDecoder(buf).Decode(&conf); err != nil { + decoder := json.NewDecoder(buf) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&conf); err != nil { return nil, fmt.Errorf("failure to decode config: %w", err) } return &conf, nil From b69676b2723e007035880d68a073ba4009f7d27d Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 14:44:33 +0300 Subject: [PATCH 02/28] Add changelog entry. --- docs/changelogs/v0.33.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changelogs/v0.33.md b/docs/changelogs/v0.33.md index ba7aa9f4f33..aabd19be9c7 100644 --- a/docs/changelogs/v0.33.md +++ b/docs/changelogs/v0.33.md @@ -18,6 +18,7 @@ - [New DoH resolvers for non-ICANN DNSLinks](#new-doh-resolvers-for-non-icann-dnslinks) - [๐Ÿ“ฆ๏ธ Important dependency updates](#-important-dependency-updates) - [Escape Redirect URL for Directory](#escape-redirect-url-for-directory) + - [JSON config validation](#json-config-validation) - [๐Ÿ“ Changelog](#-changelog) - [๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors](#-contributors) @@ -112,6 +113,10 @@ We have fixed a number of issues that were triggered by writing or copying many When navigating to a subdirectory, served by the Kubo web server, a subdirectory without a trailing slash gets redirected to a URL with a trailing slash. If there are special characters such as "%" in the subdirectory name then these must be escaped in the redirect URL. Previously this was not being done and was preventing navigation to such subdirectories, requiring the user to manually add a trailing slash to the subdirectory URL. This is now fixed to handle the redirect to URLs with characters that must be escaped. +### JSON config validation + +Fixed an issue concerning the lack of proper validation when updating the config with `config`. + ### ๐Ÿ“ Changelog ### ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors From 42692c622d08739207ccd3ae7469692233e52db2 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 17:31:15 +0300 Subject: [PATCH 03/28] Fix test --- test/sharness/t0070-user-config.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/sharness/t0070-user-config.sh b/test/sharness/t0070-user-config.sh index 63c26ea3afb..1dc4c0369a0 100755 --- a/test/sharness/t0070-user-config.sh +++ b/test/sharness/t0070-user-config.sh @@ -11,10 +11,12 @@ test_description="Test user-provided config values" test_init_ipfs test_expect_success "bootstrap doesn't overwrite user-provided config keys (top-level)" ' - ipfs config Foo.Bar baz && + ipfs config Provider.Strategy >previous && + ipfs config Provider.Strategy foo && ipfs bootstrap rm --all && - echo "baz" >expected && - ipfs config Foo.Bar >actual && + echo "foo" >expected && + ipfs config Provider.Strategy >actual && + ipfs config Provider.Strategy $(cat previous) && test_cmp expected actual ' From f589555701bcdb4a0f2f5b8892f49b05a8d381fb Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 18:59:59 +0300 Subject: [PATCH 04/28] Fix more tests, 53 still failing --- test/sharness/actual | 148 ++++++++++++++++++++++++++++++++++ test/sharness/t0021-config.sh | 63 +++++++-------- 2 files changed, 177 insertions(+), 34 deletions(-) create mode 100644 test/sharness/actual diff --git a/test/sharness/actual b/test/sharness/actual new file mode 100644 index 00000000000..9d5fa017580 --- /dev/null +++ b/test/sharness/actual @@ -0,0 +1,148 @@ +{ + "API": { + "HTTPHeaders": {} + }, + "Addresses": { + "API": "/ip4/127.0.0.1/tcp/5001", + "Announce": [], + "AppendAnnounce": [], + "Gateway": "/ip4/127.0.0.1/tcp/8080", + "NoAnnounce": [], + "Swarm": [ + "/ip4/0.0.0.0/tcp/4001", + "/ip6/::/tcp/4001", + "/ip4/0.0.0.0/udp/4001/webrtc-direct", + "/ip4/0.0.0.0/udp/4001/quic-v1", + "/ip4/0.0.0.0/udp/4001/quic-v1/webtransport", + "/ip6/::/udp/4001/webrtc-direct", + "/ip6/::/udp/4001/quic-v1", + "/ip6/::/udp/4001/quic-v1/webtransport" + ] + }, + "AutoNAT": {}, + "AutoTLS": {}, + "Bootstrap": null, + "DNS": { + "Resolvers": {} + }, + "Datastore": { + "BlockKeyCacheSize": null, + "BloomFilterSize": 0, + "GCPeriod": "1h", + "HashOnRead": false, + "Spec": { + "mounts": [ + { + "child": { + "path": "blocks", + "shardFunc": "/repo/flatfs/shard/v1/next-to-last/2", + "sync": false, + "type": "flatfs" + }, + "mountpoint": "/blocks", + "prefix": "flatfs.datastore", + "type": "measure" + }, + { + "child": { + "compression": "none", + "path": "datastore", + "type": "levelds" + }, + "mountpoint": "/", + "prefix": "leveldb.datastore", + "type": "measure" + } + ], + "type": "mount" + }, + "StorageGCWatermark": 90, + "StorageMax": "10GB" + }, + "Discovery": { + "MDNS": { + "Enabled": true + } + }, + "Experimental": { + "FilestoreEnabled": false, + "Libp2pStreamMounting": false, + "OptimisticProvide": false, + "OptimisticProvideJobsPoolSize": 0, + "P2pHttpProxy": false, + "StrategicProviding": false, + "UrlstoreEnabled": false + }, + "Gateway": { + "DeserializedResponses": null, + "DisableHTMLErrors": null, + "ExposeRoutingAPI": null, + "HTTPHeaders": {}, + "NoDNSLink": false, + "NoFetch": false, + "PublicGateways": null, + "RootRedirect": "" + }, + "Identity": { + "PeerID": "12D3KooWKr2Z9EN4jg1kzxzb8dtn21U7HFMEU1SCPawMavyH1vhR" + }, + "Import": { + "BatchMaxNodes": null, + "BatchMaxSize": null, + "CidVersion": null, + "HashFunction": null, + "UnixFSChunker": null, + "UnixFSRawLeaves": null + }, + "Internal": {}, + "Ipns": { + "RecordLifetime": "", + "RepublishPeriod": "", + "ResolveCacheSize": 128 + }, + "Migration": { + "DownloadSources": [], + "Keep": "" + }, + "Mounts": { + "FuseAllowOther": false, + "IPFS": "/ipfs", + "IPNS": "/ipns" + }, + "Peering": { + "Peers": null + }, + "Pinning": { + "RemoteServices": {} + }, + "Plugins": { + "Plugins": null + }, + "Provider": { + "Strategy": "" + }, + "Pubsub": { + "DisableSigning": false, + "Router": "" + }, + "Reprovider": {}, + "Routing": { + "Methods": null, + "Routers": null + }, + "Swarm": { + "AddrFilters": null, + "ConnMgr": {}, + "DisableBandwidthMetrics": false, + "DisableNatPortMap": false, + "RelayClient": {}, + "RelayService": {}, + "ResourceMgr": {}, + "Transports": { + "Multiplexers": {}, + "Network": {}, + "Security": {} + } + }, + "Version": {} +} diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 95a8a7d8746..fe44ed1905a 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -13,27 +13,27 @@ test_config_cmd_set() { cfg_key=$1 cfg_val=$2 - test_expect_success "ipfs config succeeds" ' - ipfs config $cfg_flags "$cfg_key" "$cfg_val" - ' + test_expect_success "ipfs config succeeds" " + ipfs config $cfg_flags \"$cfg_key\" \"$cfg_val\" + " - test_expect_success "ipfs config output looks good" ' - echo "$cfg_val" >expected && - ipfs config "$cfg_key" >actual && + test_expect_success "ipfs config output looks good" " + echo \"$cfg_val\" >expected && + ipfs config \"$cfg_key\" >actual && test_cmp expected actual - ' + " # also test our lib function. it should work too. cfg_key="Lib.$cfg_key" - test_expect_success "test_config_set succeeds" ' - test_config_set $cfg_flags "$cfg_key" "$cfg_val" - ' + test_expect_success "test_config_set succeeds" " + test_config_set $cfg_flags \"$cfg_key\" \"$cfg_val\" + " - test_expect_success "test_config_set value looks good" ' - echo "$cfg_val" >expected && - ipfs config "$cfg_key" >actual && + test_expect_success "test_config_set value looks good" " + echo \"$cfg_val\" >expected && + ipfs config \"$cfg_key\" >actual && test_cmp expected actual - ' + " } # this is a bit brittle. the problem is we need to test @@ -41,12 +41,7 @@ test_config_cmd_set() { # (i.e. just setting 'ipfs config --json foo "[1, 2, 3]"') may # set it as astring instead of proper json. We leverage the # unmarshalling that has to happen. -CONFIG_SET_JSON_TEST='{ - "MDNS": { - "Enabled": true, - "Interval": 10 - } -}' +CONFIG_SET_JSON_TEST='{"MDNS":{"Enabled":true}}' test_profile_apply_revert() { profile=$1 @@ -87,27 +82,27 @@ test_profile_apply_dry_run_not_alter() { } test_config_cmd() { - test_config_cmd_set "beep" "boop" - test_config_cmd_set "beep1" "boop2" - test_config_cmd_set "beep1" "boop2" - test_config_cmd_set "--bool" "beep2" "true" - test_config_cmd_set "--bool" "beep2" "false" - test_config_cmd_set "--json" "beep3" "true" - test_config_cmd_set "--json" "beep3" "false" + test_config_cmd_set "Addresses.API" "foo" + test_config_cmd_set "Addresses.Gateway" "bar" + test_config_cmd_set "Datastore.GCPeriod" "baz" + test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "true" + test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "false" + test_config_cmd_set "--json" "Datastore.HashOnRead" "true" + test_config_cmd_set "--json" "Datastore.HashOnRead" "false" test_config_cmd_set "--json" "Discovery" "$CONFIG_SET_JSON_TEST" - test_config_cmd_set "--json" "deep-not-defined.prop" "true" - test_config_cmd_set "--json" "deep-null" "null" - test_config_cmd_set "--json" "deep-null.prop" "true" + test_config_cmd_set "--json" "Experimental.FilestoreEnabled" "true" + test_config_cmd_set "--json" "Import.BatchMaxSize" "null" + test_config_cmd_set "--json" "Datastore.Spec" "true" test_expect_success "'ipfs config show' works" ' ipfs config show >actual ' test_expect_success "'ipfs config show' output looks good" ' - grep "\"beep\": \"boop\"," actual && - grep "\"beep1\": \"boop2\"," actual && - grep "\"beep2\": false," actual && - grep "\"beep3\": false," actual + grep "\"API\": \"foo\"," actual && + grep "\"Gateway\": \"bar\"" actual && + grep "\"Enabled\": false" actual && + grep "\"HashOnRead\": false" actual ' test_expect_success "'ipfs config show --config-file' works" ' From c35ee6649e2af40b4260a72f908252ac058b96e3 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 19:21:48 +0300 Subject: [PATCH 05/28] Remove JSON test due to fragilty, fix some more tests, Lib tests left. --- test/sharness/t0021-config.sh | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index fe44ed1905a..688fab2d8a2 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -36,13 +36,6 @@ test_config_cmd_set() { " } -# this is a bit brittle. the problem is we need to test -# with something that will be forced to unmarshal as a struct. -# (i.e. just setting 'ipfs config --json foo "[1, 2, 3]"') may -# set it as astring instead of proper json. We leverage the -# unmarshalling that has to happen. -CONFIG_SET_JSON_TEST='{"MDNS":{"Enabled":true}}' - test_profile_apply_revert() { profile=$1 inverse_profile=$2 @@ -89,10 +82,9 @@ test_config_cmd() { test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "false" test_config_cmd_set "--json" "Datastore.HashOnRead" "true" test_config_cmd_set "--json" "Datastore.HashOnRead" "false" - test_config_cmd_set "--json" "Discovery" "$CONFIG_SET_JSON_TEST" test_config_cmd_set "--json" "Experimental.FilestoreEnabled" "true" test_config_cmd_set "--json" "Import.BatchMaxSize" "null" - test_config_cmd_set "--json" "Datastore.Spec" "true" + test_config_cmd_set "--json" "Import.UnixFSRawLeaves" "true" test_expect_success "'ipfs config show' works" ' ipfs config show >actual From dd9627ea465f7b1c23156a2a527ac9681697156c Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 21:59:47 +0300 Subject: [PATCH 06/28] Remove lib tests as they don't seem to be relevant anymore. --- test/sharness/t0021-config.sh | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 688fab2d8a2..5d098ac3a0a 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -22,18 +22,6 @@ test_config_cmd_set() { ipfs config \"$cfg_key\" >actual && test_cmp expected actual " - - # also test our lib function. it should work too. - cfg_key="Lib.$cfg_key" - test_expect_success "test_config_set succeeds" " - test_config_set $cfg_flags \"$cfg_key\" \"$cfg_val\" - " - - test_expect_success "test_config_set value looks good" " - echo \"$cfg_val\" >expected && - ipfs config \"$cfg_key\" >actual && - test_cmp expected actual - " } test_profile_apply_revert() { From f7b44d013b00b542c763abf0defde393d66e873a Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 28 Jan 2025 22:15:13 +0300 Subject: [PATCH 07/28] Move changelog into v0.34 --- docs/changelogs/v0.33.md | 5 ----- docs/changelogs/v0.34.md | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 docs/changelogs/v0.34.md diff --git a/docs/changelogs/v0.33.md b/docs/changelogs/v0.33.md index aabd19be9c7..ba7aa9f4f33 100644 --- a/docs/changelogs/v0.33.md +++ b/docs/changelogs/v0.33.md @@ -18,7 +18,6 @@ - [New DoH resolvers for non-ICANN DNSLinks](#new-doh-resolvers-for-non-icann-dnslinks) - [๐Ÿ“ฆ๏ธ Important dependency updates](#-important-dependency-updates) - [Escape Redirect URL for Directory](#escape-redirect-url-for-directory) - - [JSON config validation](#json-config-validation) - [๐Ÿ“ Changelog](#-changelog) - [๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors](#-contributors) @@ -113,10 +112,6 @@ We have fixed a number of issues that were triggered by writing or copying many When navigating to a subdirectory, served by the Kubo web server, a subdirectory without a trailing slash gets redirected to a URL with a trailing slash. If there are special characters such as "%" in the subdirectory name then these must be escaped in the redirect URL. Previously this was not being done and was preventing navigation to such subdirectories, requiring the user to manually add a trailing slash to the subdirectory URL. This is now fixed to handle the redirect to URLs with characters that must be escaped. -### JSON config validation - -Fixed an issue concerning the lack of proper validation when updating the config with `config`. - ### ๐Ÿ“ Changelog ### ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors diff --git a/docs/changelogs/v0.34.md b/docs/changelogs/v0.34.md new file mode 100644 index 00000000000..af2432cb9f3 --- /dev/null +++ b/docs/changelogs/v0.34.md @@ -0,0 +1,23 @@ +# Kubo changelog v0.34 + +- [v0.34.0](#v0340) + +## v0.34.0 + +- [Overview](#overview) +- [๐Ÿ”ฆ Highlights](#-highlights) + - [JSON config validation](#json-config-validation) +- [๐Ÿ“ Changelog](#-changelog) +- [๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors](#-contributors) + +### Overview + +### ๐Ÿ”ฆ Highlights + +### JSON config validation + +Fixed an issue concerning the lack of proper validation when updating the config with `config`. + +### ๐Ÿ“ Changelog + +### ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors From 11f7f0678524dc52f47c9e77525db12f830572e4 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Wed, 29 Jan 2025 09:32:46 +0300 Subject: [PATCH 08/28] Fix config validation. --- config/config.go | 37 ++++++++++++++++++++++++++++++++++--- repo/fsrepo/fsrepo.go | 6 ++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index 0b75fe5cbc1..02a93b7f8a8 100644 --- a/config/config.go +++ b/config/config.go @@ -119,9 +119,7 @@ func FromMap(v map[string]interface{}) (*Config, error) { return nil, err } var conf Config - decoder := json.NewDecoder(buf) - decoder.DisallowUnknownFields() - if err := decoder.Decode(&conf); err != nil { + if err := json.NewDecoder(buf).Decode(&conf); err != nil { return nil, fmt.Errorf("failure to decode config: %w", err) } return &conf, nil @@ -154,3 +152,36 @@ func (c *Config) Clone() (*Config, error) { return &newConfig, nil } + +// Check if the provided key is present in the structure. +func CheckKey(key string) error { + confmap, err := ToMap(&Config{}) + if err != nil { + return err + } + var ok bool + var mcursor map[string]interface{} + var cursor interface{} = confmap + + parts := strings.Split(key, ".") + for i, part := range parts { + sofar := strings.Join(parts[:i], ".") + + mcursor, ok = cursor.(map[string]interface{}) + if !ok { + return fmt.Errorf("%s key is not a map", sofar) + } + + cursor, ok = mcursor[part] + if !ok { + // Construct the current path traversed to print a nice error message + var path string + if len(sofar) > 0 { + path += sofar + "." + } + path += part + return fmt.Errorf("%s not found", path) + } + } + return nil +} diff --git a/repo/fsrepo/fsrepo.go b/repo/fsrepo/fsrepo.go index b21b555cf7d..e7577f9dc19 100644 --- a/repo/fsrepo/fsrepo.go +++ b/repo/fsrepo/fsrepo.go @@ -676,6 +676,12 @@ func (r *FSRepo) SetConfigKey(key string, value interface{}) error { return errors.New("repo is closed") } + // Validate the key's presence in the config structure. + err := config.CheckKey(key) + if err != nil { + return err + } + // Load into a map so we don't end up writing any additional defaults to the config file. var mapconf map[string]interface{} if err := serialize.ReadConfigFile(r.configFilePath, &mapconf); err != nil { From ef3ff89ad2c33f260733b63cdc20ca4e7a59f808 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Wed, 29 Jan 2025 09:35:03 +0300 Subject: [PATCH 09/28] Delete test file --- test/sharness/actual | 148 ------------------------------------------- 1 file changed, 148 deletions(-) delete mode 100644 test/sharness/actual diff --git a/test/sharness/actual b/test/sharness/actual deleted file mode 100644 index 9d5fa017580..00000000000 --- a/test/sharness/actual +++ /dev/null @@ -1,148 +0,0 @@ -{ - "API": { - "HTTPHeaders": {} - }, - "Addresses": { - "API": "/ip4/127.0.0.1/tcp/5001", - "Announce": [], - "AppendAnnounce": [], - "Gateway": "/ip4/127.0.0.1/tcp/8080", - "NoAnnounce": [], - "Swarm": [ - "/ip4/0.0.0.0/tcp/4001", - "/ip6/::/tcp/4001", - "/ip4/0.0.0.0/udp/4001/webrtc-direct", - "/ip4/0.0.0.0/udp/4001/quic-v1", - "/ip4/0.0.0.0/udp/4001/quic-v1/webtransport", - "/ip6/::/udp/4001/webrtc-direct", - "/ip6/::/udp/4001/quic-v1", - "/ip6/::/udp/4001/quic-v1/webtransport" - ] - }, - "AutoNAT": {}, - "AutoTLS": {}, - "Bootstrap": null, - "DNS": { - "Resolvers": {} - }, - "Datastore": { - "BlockKeyCacheSize": null, - "BloomFilterSize": 0, - "GCPeriod": "1h", - "HashOnRead": false, - "Spec": { - "mounts": [ - { - "child": { - "path": "blocks", - "shardFunc": "/repo/flatfs/shard/v1/next-to-last/2", - "sync": false, - "type": "flatfs" - }, - "mountpoint": "/blocks", - "prefix": "flatfs.datastore", - "type": "measure" - }, - { - "child": { - "compression": "none", - "path": "datastore", - "type": "levelds" - }, - "mountpoint": "/", - "prefix": "leveldb.datastore", - "type": "measure" - } - ], - "type": "mount" - }, - "StorageGCWatermark": 90, - "StorageMax": "10GB" - }, - "Discovery": { - "MDNS": { - "Enabled": true - } - }, - "Experimental": { - "FilestoreEnabled": false, - "Libp2pStreamMounting": false, - "OptimisticProvide": false, - "OptimisticProvideJobsPoolSize": 0, - "P2pHttpProxy": false, - "StrategicProviding": false, - "UrlstoreEnabled": false - }, - "Gateway": { - "DeserializedResponses": null, - "DisableHTMLErrors": null, - "ExposeRoutingAPI": null, - "HTTPHeaders": {}, - "NoDNSLink": false, - "NoFetch": false, - "PublicGateways": null, - "RootRedirect": "" - }, - "Identity": { - "PeerID": "12D3KooWKr2Z9EN4jg1kzxzb8dtn21U7HFMEU1SCPawMavyH1vhR" - }, - "Import": { - "BatchMaxNodes": null, - "BatchMaxSize": null, - "CidVersion": null, - "HashFunction": null, - "UnixFSChunker": null, - "UnixFSRawLeaves": null - }, - "Internal": {}, - "Ipns": { - "RecordLifetime": "", - "RepublishPeriod": "", - "ResolveCacheSize": 128 - }, - "Migration": { - "DownloadSources": [], - "Keep": "" - }, - "Mounts": { - "FuseAllowOther": false, - "IPFS": "/ipfs", - "IPNS": "/ipns" - }, - "Peering": { - "Peers": null - }, - "Pinning": { - "RemoteServices": {} - }, - "Plugins": { - "Plugins": null - }, - "Provider": { - "Strategy": "" - }, - "Pubsub": { - "DisableSigning": false, - "Router": "" - }, - "Reprovider": {}, - "Routing": { - "Methods": null, - "Routers": null - }, - "Swarm": { - "AddrFilters": null, - "ConnMgr": {}, - "DisableBandwidthMetrics": false, - "DisableNatPortMap": false, - "RelayClient": {}, - "RelayService": {}, - "ResourceMgr": {}, - "Transports": { - "Multiplexers": {}, - "Network": {}, - "Security": {} - } - }, - "Version": {} -} From 7b88c81195718b93cc572c2316af7aed8155115b Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Wed, 29 Jan 2025 12:31:31 +0300 Subject: [PATCH 10/28] Use reflection to ignore encoding/json tags. --- config/config.go | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index 02a93b7f8a8..b9a2ca6f612 100644 --- a/config/config.go +++ b/config/config.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "strings" "github.com/ipfs/kubo/misc/fsutil" @@ -137,6 +138,27 @@ func ToMap(conf *Config) (map[string]interface{}, error) { return m, nil } +// Convert config to a map, without using encoding/json. +func ReflectToMap(conf interface{}) map[string]interface{} { + confmap := make(map[string]interface{}) + rv := reflect.ValueOf(conf) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() + } + + for i := 0; i < rv.NumField(); i++ { + field := rv.Field(i) + if field.CanInterface() { + if field.Kind() == reflect.Struct { + confmap[rv.Type().Field(i).Name] = ReflectToMap(field.Interface()) + } else { + confmap[rv.Type().Field(i).Name] = field.Interface() + } + } + } + return confmap +} + // Clone copies the config. Use when updating. func (c *Config) Clone() (*Config, error) { var newConfig Config @@ -155,10 +177,12 @@ func (c *Config) Clone() (*Config, error) { // Check if the provided key is present in the structure. func CheckKey(key string) error { - confmap, err := ToMap(&Config{}) - if err != nil { - return err - } + conf := Config{} + + // Convert an empty config to a map without JSON. + confmap := ReflectToMap(&conf) + + // Parse the key and verify it's presence in the map. var ok bool var mcursor map[string]interface{} var cursor interface{} = confmap From 2765ec0315a76ceafd91ddfd9db344801162bccb Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Wed, 29 Jan 2025 21:34:18 +0300 Subject: [PATCH 11/28] Add exceptions for fields that were already maps in the config, fix one more test. --- config/config.go | 6 ++++++ test/sharness/t0002-docker-image.sh | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index b9a2ca6f612..4031e7d8d6e 100644 --- a/config/config.go +++ b/config/config.go @@ -151,6 +151,8 @@ func ReflectToMap(conf interface{}) map[string]interface{} { if field.CanInterface() { if field.Kind() == reflect.Struct { confmap[rv.Type().Field(i).Name] = ReflectToMap(field.Interface()) + } else if field.Kind() == reflect.Map { + confmap[rv.Type().Field(i).Name] = "map" } else { confmap[rv.Type().Field(i).Name] = field.Interface() } @@ -193,6 +195,10 @@ func CheckKey(key string) error { mcursor, ok = cursor.(map[string]interface{}) if !ok { + s, ok := cursor.(string) + if ok && s == "map" { + return nil + } return fmt.Errorf("%s key is not a map", sofar) } diff --git a/test/sharness/t0002-docker-image.sh b/test/sharness/t0002-docker-image.sh index 2ff827806ba..70c68ab4049 100755 --- a/test/sharness/t0002-docker-image.sh +++ b/test/sharness/t0002-docker-image.sh @@ -36,8 +36,8 @@ test_expect_success "docker image build succeeds" ' ' test_expect_success "write init scripts" ' - echo "ipfs config Foo Bar" > 001.sh && - echo "ipfs config Baz Qux" > 002.sh && + echo "ipfs config Provider.Strategy Bar" > 001.sh && + echo "ipfs config Datastore.Path Qux" > 002.sh && chmod +x 002.sh ' From 354256ab28e4e6298ed6e9b025873e92940003e7 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Wed, 29 Jan 2025 21:53:37 +0300 Subject: [PATCH 12/28] Fix last (hopefully) test --- test/sharness/t0002-docker-image.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sharness/t0002-docker-image.sh b/test/sharness/t0002-docker-image.sh index 70c68ab4049..dfe6bb022fb 100755 --- a/test/sharness/t0002-docker-image.sh +++ b/test/sharness/t0002-docker-image.sh @@ -65,10 +65,10 @@ test_expect_success "check that init scripts were run correctly and in the corre test_expect_success "check that init script configs were applied" ' echo Bar > expected && - docker exec "$DOC_ID" ipfs config Foo > actual && + docker exec "$DOC_ID" ipfs config Provider.Strategy > actual && test_cmp actual expected && echo Qux > expected && - docker exec "$DOC_ID" ipfs config Baz > actual && + docker exec "$DOC_ID" ipfs config Datastore.Path > actual && test_cmp actual expected ' From 12a018522c04eeeba5ff074a429ee2238fc265e6 Mon Sep 17 00:00:00 2001 From: galargh Date: Wed, 29 Jan 2025 20:13:36 +0100 Subject: [PATCH 13/28] test: fix the socat tests after the ubuntu 24.04 upgrade --- test/sharness/t0060-daemon.sh | 6 +++--- test/sharness/t0061-daemon-opts.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/sharness/t0060-daemon.sh b/test/sharness/t0060-daemon.sh index 431ff245ca3..2a25f5852ab 100755 --- a/test/sharness/t0060-daemon.sh +++ b/test/sharness/t0060-daemon.sh @@ -131,21 +131,21 @@ test_expect_success "ipfs help output looks good" ' # check transport is encrypted by default and no plaintext is allowed test_expect_success SOCAT "default transport should support encryption (TLS, needs socat )" ' - socat - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-tls && + socat -s - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-tls && grep -q "/tls" swarmnc && test_must_fail grep -q "na" swarmnc || test_fsh cat swarmnc ' test_expect_success SOCAT "default transport should support encryption (Noise, needs socat )" ' - socat - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-noise && + socat -s - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-noise && grep -q "/noise" swarmnc && test_must_fail grep -q "na" swarmnc || test_fsh cat swarmnc ' test_expect_success SOCAT "default transport should not support plaintext (needs socat )" ' - socat - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-plaintext && + socat -s - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-plaintext && grep -q "na" swarmnc && test_must_fail grep -q "/plaintext" swarmnc || test_fsh cat swarmnc diff --git a/test/sharness/t0061-daemon-opts.sh b/test/sharness/t0061-daemon-opts.sh index 531d2d247a5..a168ae4b068 100755 --- a/test/sharness/t0061-daemon-opts.sh +++ b/test/sharness/t0061-daemon-opts.sh @@ -18,7 +18,7 @@ apiaddr=$API_ADDR # Odd. this fails here, but the inverse works on t0060-daemon. test_expect_success SOCAT 'transport should be unencrypted ( needs socat )' ' - socat - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-plaintext && + socat -s - tcp:localhost:$SWARM_PORT,connect-timeout=1 > swarmnc < ../t0060-data/mss-plaintext && grep -q "/plaintext" swarmnc && test_must_fail grep -q "na" swarmnc || test_fsh cat swarmnc From af0c5c6d286d86fcb9cc6f83513e46b2c7bb156f Mon Sep 17 00:00:00 2001 From: guillaumemichel Date: Thu, 30 Jan 2025 16:39:28 +0100 Subject: [PATCH 14/28] docker sharness --- test/sharness/t0002-docker-image.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sharness/t0002-docker-image.sh b/test/sharness/t0002-docker-image.sh index dfe6bb022fb..8812c277a74 100755 --- a/test/sharness/t0002-docker-image.sh +++ b/test/sharness/t0002-docker-image.sh @@ -37,7 +37,7 @@ test_expect_success "docker image build succeeds" ' test_expect_success "write init scripts" ' echo "ipfs config Provider.Strategy Bar" > 001.sh && - echo "ipfs config Datastore.Path Qux" > 002.sh && + echo "ipfs config Pubsub.Router Qux" > 002.sh && chmod +x 002.sh ' @@ -68,7 +68,7 @@ test_expect_success "check that init script configs were applied" ' docker exec "$DOC_ID" ipfs config Provider.Strategy > actual && test_cmp actual expected && echo Qux > expected && - docker exec "$DOC_ID" ipfs config Datastore.Path > actual && + docker exec "$DOC_ID" ipfs config Pubsub.Router > actual && test_cmp actual expected ' From 7a3732f5882dd77afb12859d9ac194eb10eda8e8 Mon Sep 17 00:00:00 2001 From: guillaumemichel Date: Fri, 31 Jan 2025 15:01:52 +0100 Subject: [PATCH 15/28] updated ReflectToMap --- config/config.go | 71 +++++++++++++++++++++++-------- config/config_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 17 deletions(-) diff --git a/config/config.go b/config/config.go index 4031e7d8d6e..7519528970e 100644 --- a/config/config.go +++ b/config/config.go @@ -138,27 +138,64 @@ func ToMap(conf *Config) (map[string]interface{}, error) { return m, nil } -// Convert config to a map, without using encoding/json. -func ReflectToMap(conf interface{}) map[string]interface{} { - confmap := make(map[string]interface{}) - rv := reflect.ValueOf(conf) - if rv.Kind() == reflect.Ptr { - rv = rv.Elem() +// Convert config to a map, without using encoding/json, since +// zero/empty/'omitempty' fields are exclused by encoding/json during +// marshaling. +func ReflectToMap(conf interface{}) interface{} { + v := reflect.ValueOf(conf) + if !v.IsValid() { + return nil } - for i := 0; i < rv.NumField(); i++ { - field := rv.Field(i) - if field.CanInterface() { - if field.Kind() == reflect.Struct { - confmap[rv.Type().Field(i).Name] = ReflectToMap(field.Interface()) - } else if field.Kind() == reflect.Map { - confmap[rv.Type().Field(i).Name] = "map" - } else { - confmap[rv.Type().Field(i).Name] = field.Interface() + // Handle pointer type + if v.Kind() == reflect.Ptr { + if v.IsNil() { + // Create a zero value of the pointer's element type + elemType := v.Type().Elem() + zero := reflect.Zero(elemType) + return ReflectToMap(zero.Interface()) + } + v = v.Elem() + } + + switch v.Kind() { + case reflect.Struct: + result := make(map[string]interface{}) + t := v.Type() + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + // Only include exported fields + if field.CanInterface() { + result[t.Field(i).Name] = ReflectToMap(field.Interface()) } } + return result + + case reflect.Map: + result := make(map[string]interface{}) + iter := v.MapRange() + for iter.Next() { + key := iter.Key() + // Convert map keys to strings for consistency + keyStr := fmt.Sprint(ReflectToMap(key.Interface())) + result[keyStr] = ReflectToMap(iter.Value().Interface()) + } + return result + + case reflect.Slice, reflect.Array: + result := make([]interface{}, v.Len()) + for i := 0; i < v.Len(); i++ { + result[i] = ReflectToMap(v.Index(i).Interface()) + } + return result + + default: + // For basic types (int, string, etc.), just return the value + if v.CanInterface() { + return v.Interface() + } + return nil } - return confmap } // Clone copies the config. Use when updating. @@ -187,7 +224,7 @@ func CheckKey(key string) error { // Parse the key and verify it's presence in the map. var ok bool var mcursor map[string]interface{} - var cursor interface{} = confmap + cursor := confmap parts := strings.Split(key, ".") for i, part := range parts { diff --git a/config/config_test.go b/config/config_test.go index dead06f8a23..9e166a606af 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -27,3 +27,102 @@ func TestClone(t *testing.T) { t.Fatal("HTTP headers not preserved") } } + +func TestReflectToMap(t *testing.T) { + // Helper function to create a test config with various field types + reflectedConfig := ReflectToMap(new(Config)) + + mapConfig, ok := reflectedConfig.(map[string]interface{}) + if !ok { + t.Fatal("Config didn't convert to map") + } + + reflectedIdentity, ok := mapConfig["Identity"] + if !ok { + t.Fatal("Identity field not found") + } + + mapIdentity, ok := reflectedIdentity.(map[string]interface{}) + if !ok { + t.Fatal("Identity field didn't convert to map") + } + + // Test string field reflection + reflectedPeerID, ok := mapIdentity["PeerID"] + if !ok { + t.Fatal("PeerID field not found in Identity") + } + if _, ok := reflectedPeerID.(string); !ok { + t.Fatal("PeerID field didn't convert to string") + } + + // Test omitempty json string field + reflectedPrivKey, ok := mapIdentity["PrivKey"] + if !ok { + t.Fatal("PrivKey omitempty field not found in Identity") + } + if _, ok := reflectedPrivKey.(string); !ok { + t.Fatal("PrivKey omitempty field didn't convert to string") + } + + // Test slices field + reflectedBootstrap, ok := mapConfig["Bootstrap"] + if !ok { + t.Fatal("Bootstrap field not found in config") + } + bootstrap, ok := reflectedBootstrap.([]interface{}) + if !ok { + t.Fatal("Bootstrap field didn't convert to []string") + } + if len(bootstrap) != 0 { + t.Fatal("Bootstrap len is incorrect") + } + + reflectedDatastore, ok := mapConfig["Datastore"] + if !ok { + t.Fatal("Datastore field not found in config") + } + datastore, ok := reflectedDatastore.(map[string]interface{}) + if !ok { + t.Fatal("Datastore field didn't convert to map") + } + storageGCWatermark, ok := datastore["StorageGCWatermark"] + if !ok { + t.Fatal("StorageGCWatermark field not found in Datastore") + } + // Test int field + if _, ok := storageGCWatermark.(int64); !ok { + t.Fatal("StorageGCWatermark field didn't convert to int64") + } + noSync, ok := datastore["NoSync"] + if !ok { + t.Fatal("NoSync field not found in Datastore") + } + // Test bool field + if _, ok := noSync.(bool); !ok { + t.Fatal("NoSync field didn't convert to bool") + } + + reflectedDNS, ok := mapConfig["DNS"] + if !ok { + t.Fatal("DNS field not found in config") + } + DNS, ok := reflectedDNS.(map[string]interface{}) + if !ok { + t.Fatal("DNS field didn't convert to map") + } + reflectedResolvers, ok := DNS["Resolvers"] + if !ok { + t.Fatal("Resolvers field not found in DNS") + } + // Test map field + if _, ok := reflectedResolvers.(map[string]interface{}); !ok { + t.Fatal("Resolvers field didn't convert to map") + } + + // Test pointer field + if _, ok := DNS["MaxCacheTTL"].(map[string]interface{}); !ok { + // Since OptionalDuration only field is private, we cannot test it + t.Fatal("MaxCacheTTL field didn't convert to map") + } +} From 90869710ca7ecb57c10eda44dc1a53487fe7c38b Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Fri, 31 Jan 2025 18:22:38 +0300 Subject: [PATCH 16/28] Fix nested maps and write a test for it. --- config/config.go | 115 ++++++++++++++++------------------ config/config_test.go | 6 +- test/sharness/t0021-config.sh | 11 +++- 3 files changed, 66 insertions(+), 66 deletions(-) diff --git a/config/config.go b/config/config.go index 7519528970e..cf6cdcb2d9b 100644 --- a/config/config.go +++ b/config/config.go @@ -138,66 +138,6 @@ func ToMap(conf *Config) (map[string]interface{}, error) { return m, nil } -// Convert config to a map, without using encoding/json, since -// zero/empty/'omitempty' fields are exclused by encoding/json during -// marshaling. -func ReflectToMap(conf interface{}) interface{} { - v := reflect.ValueOf(conf) - if !v.IsValid() { - return nil - } - - // Handle pointer type - if v.Kind() == reflect.Ptr { - if v.IsNil() { - // Create a zero value of the pointer's element type - elemType := v.Type().Elem() - zero := reflect.Zero(elemType) - return ReflectToMap(zero.Interface()) - } - v = v.Elem() - } - - switch v.Kind() { - case reflect.Struct: - result := make(map[string]interface{}) - t := v.Type() - for i := 0; i < v.NumField(); i++ { - field := v.Field(i) - // Only include exported fields - if field.CanInterface() { - result[t.Field(i).Name] = ReflectToMap(field.Interface()) - } - } - return result - - case reflect.Map: - result := make(map[string]interface{}) - iter := v.MapRange() - for iter.Next() { - key := iter.Key() - // Convert map keys to strings for consistency - keyStr := fmt.Sprint(ReflectToMap(key.Interface())) - result[keyStr] = ReflectToMap(iter.Value().Interface()) - } - return result - - case reflect.Slice, reflect.Array: - result := make([]interface{}, v.Len()) - for i := 0; i < v.Len(); i++ { - result[i] = ReflectToMap(v.Index(i).Interface()) - } - return result - - default: - // For basic types (int, string, etc.), just return the value - if v.CanInterface() { - return v.Interface() - } - return nil - } -} - // Clone copies the config. Use when updating. func (c *Config) Clone() (*Config, error) { var newConfig Config @@ -219,7 +159,7 @@ func CheckKey(key string) error { conf := Config{} // Convert an empty config to a map without JSON. - confmap := ReflectToMap(&conf) + confmap := GetValidationMap(&conf) // Parse the key and verify it's presence in the map. var ok bool @@ -252,3 +192,56 @@ func CheckKey(key string) error { } return nil } + +// Convert config to a map, without using encoding/json, since +// zero/empty/'omitempty' fields are exclused by encoding/json during +// marshaling. +func GetValidationMap(conf interface{}) interface{} { + v := reflect.ValueOf(conf) + if !v.IsValid() { + return nil + } + + // Handle pointer type + if v.Kind() == reflect.Ptr { + if v.IsNil() { + // Create a zero value of the pointer's element type + elemType := v.Type().Elem() + zero := reflect.Zero(elemType) + return GetValidationMap(zero.Interface()) + } + v = v.Elem() + } + + switch v.Kind() { + case reflect.Struct: + result := make(map[string]interface{}) + t := v.Type() + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + // Only include exported fields + if field.CanInterface() { + result[t.Field(i).Name] = GetValidationMap(field.Interface()) + } + } + return result + + case reflect.Map: + result := "map" + return result + + case reflect.Slice, reflect.Array: + result := make([]interface{}, v.Len()) + for i := 0; i < v.Len(); i++ { + result[i] = GetValidationMap(v.Index(i).Interface()) + } + return result + + default: + // For basic types (int, string, etc.), just return the value + if v.CanInterface() { + return v.Interface() + } + return nil + } +} diff --git a/config/config_test.go b/config/config_test.go index 9e166a606af..dfa61700f35 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -28,9 +28,9 @@ func TestClone(t *testing.T) { } } -func TestReflectToMap(t *testing.T) { +func TestGetValidationMap(t *testing.T) { // Helper function to create a test config with various field types - reflectedConfig := ReflectToMap(new(Config)) + reflectedConfig := GetValidationMap(new(Config)) mapConfig, ok := reflectedConfig.(map[string]interface{}) if !ok { @@ -116,7 +116,7 @@ func TestReflectToMap(t *testing.T) { t.Fatal("Resolvers field not found in DNS") } // Test map field - if _, ok := reflectedResolvers.(map[string]interface{}); !ok { + if v, _ := reflectedResolvers.(string); v != "map" { t.Fatal("Resolvers field didn't convert to map") } diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 5d098ac3a0a..fcc4645f50a 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -19,8 +19,14 @@ test_config_cmd_set() { test_expect_success "ipfs config output looks good" " echo \"$cfg_val\" >expected && - ipfs config \"$cfg_key\" >actual && - test_cmp expected actual + if [$cfg_flags != \"--json\"]; then + ipfs config \"$cfg_key\" >actual && + test_cmp expected actual + else + ipfs config \"$cfg_key\" | tr -d \"\\n\\t \" >actual && + echo >>actual && + test_cmp expected actual + fi " } @@ -73,6 +79,7 @@ test_config_cmd() { test_config_cmd_set "--json" "Experimental.FilestoreEnabled" "true" test_config_cmd_set "--json" "Import.BatchMaxSize" "null" test_config_cmd_set "--json" "Import.UnixFSRawLeaves" "true" + test_config_cmd_set "--json" "Routing.Routers.Test" "{\\\"Parameters\\\":\\\"Test\\\",\\\"Type\\\":\\\"Test\\\"}" test_expect_success "'ipfs config show' works" ' ipfs config show >actual From 564d9dd5d29ac069be6500af12926f2304dc98ce Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Fri, 31 Jan 2025 18:41:29 +0300 Subject: [PATCH 17/28] Add tests for other data types --- test/sharness/t0021-config.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index fcc4645f50a..5ce4bc6b7c7 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -72,6 +72,7 @@ test_config_cmd() { test_config_cmd_set "Addresses.API" "foo" test_config_cmd_set "Addresses.Gateway" "bar" test_config_cmd_set "Datastore.GCPeriod" "baz" + test_config_cmd_set "AutoNAT.ServiceMode" "enabled" test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "true" test_config_cmd_set "--bool" "Discovery.MDNS.Enabled" "false" test_config_cmd_set "--json" "Datastore.HashOnRead" "true" @@ -80,6 +81,8 @@ test_config_cmd() { test_config_cmd_set "--json" "Import.BatchMaxSize" "null" test_config_cmd_set "--json" "Import.UnixFSRawLeaves" "true" test_config_cmd_set "--json" "Routing.Routers.Test" "{\\\"Parameters\\\":\\\"Test\\\",\\\"Type\\\":\\\"Test\\\"}" + test_config_cmd_set "--json" "Experimental.OptimisticProvideJobsPoolSize" "1337" + test_config_cmd_set "--json" "Addresses.Swarm" "[\\\"test\\\",\\\"test\\\",\\\"test\\\"]" test_expect_success "'ipfs config show' works" ' ipfs config show >actual From c34c0682fa690b44fe6e47003f0c68087fd89c68 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Mon, 3 Feb 2025 14:00:37 +0300 Subject: [PATCH 18/28] Revert "Fix nested maps and write a test for it." This reverts commit 90869710ca7ecb57c10eda44dc1a53487fe7c38b. --- config/config.go | 115 ++++++++++++++++++++++-------------------- config/config_test.go | 6 +-- 2 files changed, 64 insertions(+), 57 deletions(-) diff --git a/config/config.go b/config/config.go index cf6cdcb2d9b..7519528970e 100644 --- a/config/config.go +++ b/config/config.go @@ -138,6 +138,66 @@ func ToMap(conf *Config) (map[string]interface{}, error) { return m, nil } +// Convert config to a map, without using encoding/json, since +// zero/empty/'omitempty' fields are exclused by encoding/json during +// marshaling. +func ReflectToMap(conf interface{}) interface{} { + v := reflect.ValueOf(conf) + if !v.IsValid() { + return nil + } + + // Handle pointer type + if v.Kind() == reflect.Ptr { + if v.IsNil() { + // Create a zero value of the pointer's element type + elemType := v.Type().Elem() + zero := reflect.Zero(elemType) + return ReflectToMap(zero.Interface()) + } + v = v.Elem() + } + + switch v.Kind() { + case reflect.Struct: + result := make(map[string]interface{}) + t := v.Type() + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + // Only include exported fields + if field.CanInterface() { + result[t.Field(i).Name] = ReflectToMap(field.Interface()) + } + } + return result + + case reflect.Map: + result := make(map[string]interface{}) + iter := v.MapRange() + for iter.Next() { + key := iter.Key() + // Convert map keys to strings for consistency + keyStr := fmt.Sprint(ReflectToMap(key.Interface())) + result[keyStr] = ReflectToMap(iter.Value().Interface()) + } + return result + + case reflect.Slice, reflect.Array: + result := make([]interface{}, v.Len()) + for i := 0; i < v.Len(); i++ { + result[i] = ReflectToMap(v.Index(i).Interface()) + } + return result + + default: + // For basic types (int, string, etc.), just return the value + if v.CanInterface() { + return v.Interface() + } + return nil + } +} + // Clone copies the config. Use when updating. func (c *Config) Clone() (*Config, error) { var newConfig Config @@ -159,7 +219,7 @@ func CheckKey(key string) error { conf := Config{} // Convert an empty config to a map without JSON. - confmap := GetValidationMap(&conf) + confmap := ReflectToMap(&conf) // Parse the key and verify it's presence in the map. var ok bool @@ -192,56 +252,3 @@ func CheckKey(key string) error { } return nil } - -// Convert config to a map, without using encoding/json, since -// zero/empty/'omitempty' fields are exclused by encoding/json during -// marshaling. -func GetValidationMap(conf interface{}) interface{} { - v := reflect.ValueOf(conf) - if !v.IsValid() { - return nil - } - - // Handle pointer type - if v.Kind() == reflect.Ptr { - if v.IsNil() { - // Create a zero value of the pointer's element type - elemType := v.Type().Elem() - zero := reflect.Zero(elemType) - return GetValidationMap(zero.Interface()) - } - v = v.Elem() - } - - switch v.Kind() { - case reflect.Struct: - result := make(map[string]interface{}) - t := v.Type() - for i := 0; i < v.NumField(); i++ { - field := v.Field(i) - // Only include exported fields - if field.CanInterface() { - result[t.Field(i).Name] = GetValidationMap(field.Interface()) - } - } - return result - - case reflect.Map: - result := "map" - return result - - case reflect.Slice, reflect.Array: - result := make([]interface{}, v.Len()) - for i := 0; i < v.Len(); i++ { - result[i] = GetValidationMap(v.Index(i).Interface()) - } - return result - - default: - // For basic types (int, string, etc.), just return the value - if v.CanInterface() { - return v.Interface() - } - return nil - } -} diff --git a/config/config_test.go b/config/config_test.go index dfa61700f35..9e166a606af 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -28,9 +28,9 @@ func TestClone(t *testing.T) { } } -func TestGetValidationMap(t *testing.T) { +func TestReflectToMap(t *testing.T) { // Helper function to create a test config with various field types - reflectedConfig := GetValidationMap(new(Config)) + reflectedConfig := ReflectToMap(new(Config)) mapConfig, ok := reflectedConfig.(map[string]interface{}) if !ok { @@ -116,7 +116,7 @@ func TestGetValidationMap(t *testing.T) { t.Fatal("Resolvers field not found in DNS") } // Test map field - if v, _ := reflectedResolvers.(string); v != "map" { + if _, ok := reflectedResolvers.(map[string]interface{}); !ok { t.Fatal("Resolvers field didn't convert to map") } From 44fbd882d03a224b341920e64c14e40ea0ace3fb Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Mon, 3 Feb 2025 14:26:35 +0300 Subject: [PATCH 19/28] Validate structs nested in maps. --- config/config.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index 7519528970e..f020fda111d 100644 --- a/config/config.go +++ b/config/config.go @@ -180,6 +180,10 @@ func ReflectToMap(conf interface{}) interface{} { keyStr := fmt.Sprint(ReflectToMap(key.Interface())) result[keyStr] = ReflectToMap(iter.Value().Interface()) } + sample := reflect.Zero(v.Type().Elem()) + if sample.CanInterface() { + result["*"] = ReflectToMap(sample.Interface()) + } return result case reflect.Slice, reflect.Array: @@ -220,6 +224,8 @@ func CheckKey(key string) error { // Convert an empty config to a map without JSON. confmap := ReflectToMap(&conf) + b, _ := json.MarshalIndent(confmap, "", " ") + fmt.Printf("%s\n", b) // Parse the key and verify it's presence in the map. var ok bool @@ -232,15 +238,14 @@ func CheckKey(key string) error { mcursor, ok = cursor.(map[string]interface{}) if !ok { - s, ok := cursor.(string) - if ok && s == "map" { - return nil - } return fmt.Errorf("%s key is not a map", sofar) } cursor, ok = mcursor[part] if !ok { + if cursor, ok = mcursor["*"]; ok { + continue + } // Construct the current path traversed to print a nice error message var path string if len(sofar) > 0 { From 8a669cd8cc129d314dd03a1e19b3abc8f91e1a5e Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Mon, 3 Feb 2025 14:32:50 +0300 Subject: [PATCH 20/28] Add comments. --- config/config.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/config.go b/config/config.go index f020fda111d..c6930c4615f 100644 --- a/config/config.go +++ b/config/config.go @@ -180,6 +180,7 @@ func ReflectToMap(conf interface{}) interface{} { keyStr := fmt.Sprint(ReflectToMap(key.Interface())) result[keyStr] = ReflectToMap(iter.Value().Interface()) } + // Add a sample to differentiate between a map and a struct on validation. sample := reflect.Zero(v.Type().Elem()) if sample.CanInterface() { result["*"] = ReflectToMap(sample.Interface()) @@ -243,6 +244,7 @@ func CheckKey(key string) error { cursor, ok = mcursor[part] if !ok { + // If the config sections is a map, validate against the default entry. if cursor, ok = mcursor["*"]; ok { continue } From 0a2533167edee17e7e719629bd793d35ffcbc728 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Mon, 3 Feb 2025 14:34:57 +0300 Subject: [PATCH 21/28] Remove debug --- config/config.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/config.go b/config/config.go index c6930c4615f..8c932daa462 100644 --- a/config/config.go +++ b/config/config.go @@ -225,8 +225,6 @@ func CheckKey(key string) error { // Convert an empty config to a map without JSON. confmap := ReflectToMap(&conf) - b, _ := json.MarshalIndent(confmap, "", " ") - fmt.Printf("%s\n", b) // Parse the key and verify it's presence in the map. var ok bool From eae43828bd560bd2447f83f4b40532e77394d363 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Mon, 3 Feb 2025 14:44:30 +0300 Subject: [PATCH 22/28] Add test for PublicGateways --- test/sharness/t0021-config.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 5ce4bc6b7c7..52700c068fb 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -83,6 +83,7 @@ test_config_cmd() { test_config_cmd_set "--json" "Routing.Routers.Test" "{\\\"Parameters\\\":\\\"Test\\\",\\\"Type\\\":\\\"Test\\\"}" test_config_cmd_set "--json" "Experimental.OptimisticProvideJobsPoolSize" "1337" test_config_cmd_set "--json" "Addresses.Swarm" "[\\\"test\\\",\\\"test\\\",\\\"test\\\"]" + test_config_cmd_set "--json" "Gateway.PublicGateways.Foo" "{\\\"DeserializedResponses\\\":true,\\\"InlineDNSLink\\\":false,\\\"NoDNSLink\\\":false,\\\"Paths\\\":[\\\"Bar\\\",\\\"Baz\\\"],\\\"UseSubdomains\\\":true}" test_expect_success "'ipfs config show' works" ' ipfs config show >actual From 7e8d3cad45fa10f4d58fde722b548a66c80ab9e2 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Mon, 3 Feb 2025 14:47:16 +0300 Subject: [PATCH 23/28] Add test for sturct in map --- test/sharness/t0021-config.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 52700c068fb..3e688634863 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -84,6 +84,7 @@ test_config_cmd() { test_config_cmd_set "--json" "Experimental.OptimisticProvideJobsPoolSize" "1337" test_config_cmd_set "--json" "Addresses.Swarm" "[\\\"test\\\",\\\"test\\\",\\\"test\\\"]" test_config_cmd_set "--json" "Gateway.PublicGateways.Foo" "{\\\"DeserializedResponses\\\":true,\\\"InlineDNSLink\\\":false,\\\"NoDNSLink\\\":false,\\\"Paths\\\":[\\\"Bar\\\",\\\"Baz\\\"],\\\"UseSubdomains\\\":true}" + test_config_cmd_set "--bool" "Gateway.PublicGateways.Foo.UseSubdomains" "false" test_expect_success "'ipfs config show' works" ' ipfs config show >actual From bed120fd9b9b551151144aaedab278b8b161778a Mon Sep 17 00:00:00 2001 From: Guillaume Michel Date: Tue, 4 Feb 2025 10:21:37 +0100 Subject: [PATCH 24/28] comment typo --- config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 8c932daa462..552e7661c7a 100644 --- a/config/config.go +++ b/config/config.go @@ -139,7 +139,7 @@ func ToMap(conf *Config) (map[string]interface{}, error) { } // Convert config to a map, without using encoding/json, since -// zero/empty/'omitempty' fields are exclused by encoding/json during +// zero/empty/'omitempty' fields are excluded by encoding/json during // marshaling. func ReflectToMap(conf interface{}) interface{} { v := reflect.ValueOf(conf) From 69235e204f8754c0861043700ac5eb5b4ff00094 Mon Sep 17 00:00:00 2001 From: guillaumemichel Date: Tue, 4 Feb 2025 10:54:10 +0100 Subject: [PATCH 25/28] slight refactor --- config/config.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/config/config.go b/config/config.go index 552e7661c7a..b3c0575b9af 100644 --- a/config/config.go +++ b/config/config.go @@ -224,34 +224,27 @@ func CheckKey(key string) error { conf := Config{} // Convert an empty config to a map without JSON. - confmap := ReflectToMap(&conf) + cursor := ReflectToMap(&conf) // Parse the key and verify it's presence in the map. var ok bool - var mcursor map[string]interface{} - cursor := confmap + var mapCursor map[string]interface{} parts := strings.Split(key, ".") for i, part := range parts { - sofar := strings.Join(parts[:i], ".") - - mcursor, ok = cursor.(map[string]interface{}) + mapCursor, ok = cursor.(map[string]interface{}) if !ok { - return fmt.Errorf("%s key is not a map", sofar) + path := strings.Join(parts[:i], ".") + return fmt.Errorf("%s key is not a map", path) } - cursor, ok = mcursor[part] + cursor, ok = mapCursor[part] if !ok { // If the config sections is a map, validate against the default entry. - if cursor, ok = mcursor["*"]; ok { + if cursor, ok = mapCursor["*"]; ok { continue } - // Construct the current path traversed to print a nice error message - var path string - if len(sofar) > 0 { - path += sofar + "." - } - path += part + path := strings.Join(parts[:i+1], ".") return fmt.Errorf("%s not found", path) } } From 8e72a65f9e9f99b75d2c1948f5bb3cc2c4182c91 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 4 Feb 2025 14:23:02 +0300 Subject: [PATCH 26/28] Add CheckKey tests. --- config/config_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/config/config_test.go b/config/config_test.go index 9e166a606af..e99c93f7a02 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -126,3 +126,31 @@ func TestReflectToMap(t *testing.T) { t.Fatal("MaxCacheTTL field didn't convert to map") } } + +// Test validation of options set through "ipfs config" +func TestCheckKey(t *testing.T) { + err := CheckKey("Foo.Bar") + if err == nil { + t.Fatal("Foo.Bar isn't a valid key in the config") + } + + err = CheckKey("Provider.Strategy") + if err != nil { + t.Fatalf("%s: %s", err, "Provider.Strategy is a valid key in the config") + } + + err = CheckKey("Provider.Foo") + if err == nil { + t.Fatal("Provider.Foo isn't a valid key in the config") + } + + err = CheckKey("Gateway.PublicGateways.Foo.Paths") + if err != nil { + t.Fatalf("%s: %s", err, "Gateway.PublicGateways.Foo.Paths is a valid key in the config") + } + + err = CheckKey("Gateway.PublicGateways.Foo.Bar") + if err == nil { + t.Fatal("Gateway.PublicGateways.Foo.Bar isn't a valid key in the config") + } +} From 87e42d887c026f205d2a852de233338fd1672b63 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 4 Feb 2025 15:40:58 +0300 Subject: [PATCH 27/28] Add handling for interface{} in the config --- config/config.go | 5 ++++- config/config_test.go | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index b3c0575b9af..cf8b823e869 100644 --- a/config/config.go +++ b/config/config.go @@ -144,7 +144,7 @@ func ToMap(conf *Config) (map[string]interface{}, error) { func ReflectToMap(conf interface{}) interface{} { v := reflect.ValueOf(conf) if !v.IsValid() { - return nil + return "any" } // Handle pointer type @@ -234,6 +234,9 @@ func CheckKey(key string) error { for i, part := range parts { mapCursor, ok = cursor.(map[string]interface{}) if !ok { + if v, ok := cursor.(string); ok && v == "any" { + return nil + } path := strings.Join(parts[:i], ".") return fmt.Errorf("%s key is not a map", path) } diff --git a/config/config_test.go b/config/config_test.go index e99c93f7a02..d4f38f08662 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -153,4 +153,9 @@ func TestCheckKey(t *testing.T) { if err == nil { t.Fatal("Gateway.PublicGateways.Foo.Bar isn't a valid key in the config") } + + err = CheckKey("Plugins.Plugins.peerlog.Config.Enabled") + if err != nil { + t.Fatalf("%s: %s", err, "Plugins.Plugins.peerlog.Config.Enabled is a valid key in the config") + } } From 7d7633e6a53862df49253988c02257d27e4e8107 Mon Sep 17 00:00:00 2001 From: Sergey Gorbunov Date: Tue, 4 Feb 2025 18:16:11 +0300 Subject: [PATCH 28/28] "any" -> nil --- config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index cf8b823e869..3db7573d06c 100644 --- a/config/config.go +++ b/config/config.go @@ -144,7 +144,7 @@ func ToMap(conf *Config) (map[string]interface{}, error) { func ReflectToMap(conf interface{}) interface{} { v := reflect.ValueOf(conf) if !v.IsValid() { - return "any" + return nil } // Handle pointer type @@ -234,7 +234,7 @@ func CheckKey(key string) error { for i, part := range parts { mapCursor, ok = cursor.(map[string]interface{}) if !ok { - if v, ok := cursor.(string); ok && v == "any" { + if cursor == nil { return nil } path := strings.Join(parts[:i], ".")