core: add NatPortMap and default it to true

cmd/ipfs/init.go

@@ -89,7 +89,7 @@ environment variable:
 				return
 			}
 
-			conf = &config.Config{}
+			conf = config.NewDefaultConfig()
 			if err := json.NewDecoder(confFile).Decode(conf); err != nil {
 				res.SetError(err, cmds.ErrNormal)
 				return

core/builder.go

@@ -86,7 +86,7 @@ func (cfg *BuildCfg) fillDefaults() error {
 }
 
 func defaultRepo(dstore repo.Datastore) (repo.Repo, error) {
-	c := cfg.Config{}
+	c := cfg.NewDefaultConfig()
 	priv, pub, err := ci.GenerateKeyPairWithReader(ci.RSA, 1024, rand.Reader)
 	if err != nil {
 		return nil, err
@@ -109,7 +109,7 @@ func defaultRepo(dstore repo.Datastore) (repo.Repo, error) {
 
 	return &repo.Mock{
 		D: dstore,
-		C: c,
+		C: *c,
 	}, nil
 }
 

core/core.go

@@ -214,7 +214,7 @@ func (n *IpfsNode) startOnlineServices(ctx context.Context, routingOption Routin
 	}
 
 	peerhost, err := hostOption(ctx, n.Identity, n.Peerstore, n.Reporter,
-		addrfilter, tpt, protec)
+		addrfilter, tpt, protec, &ConstructPeerHostOpts{NatPortMap: cfg.Swarm.NatPortMap})
 	if err != nil {
 		return err
 	}
@@ -709,12 +709,16 @@ func listenAddresses(cfg *config.Config) ([]ma.Multiaddr, error) {
 	return listen, nil
 }
 
-type HostOption func(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, tpt smux.Transport, protc ipnet.Protector) (p2phost.Host, error)
+type ConstructPeerHostOpts struct {
+	NatPortMap bool
+}
+
+type HostOption func(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, tpt smux.Transport, protc ipnet.Protector, opts *ConstructPeerHostOpts) (p2phost.Host, error)
 
 var DefaultHostOption HostOption = constructPeerHost
 
 // isolates the complex initialization steps
-func constructPeerHost(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, tpt smux.Transport, protec ipnet.Protector) (p2phost.Host, error) {
+func constructPeerHost(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, tpt smux.Transport, protec ipnet.Protector, opts *ConstructPeerHostOpts) (p2phost.Host, error) {
 
 	// no addresses to begin with. we'll start later.
 	swrm, err := swarm.NewSwarmWithProtector(ctx, nil, id, ps, protec, tpt, bwr)
@@ -728,7 +732,12 @@ func constructPeerHost(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr
 		network.Swarm().Filters.AddDialFilter(f)
 	}
 
-	host := p2pbhost.New(network, p2pbhost.NATPortMap, bwr)
+	hostOpts := []interface{}{bwr}
+	if opts.NatPortMap {
+		hostOpts = append(hostOpts, p2pbhost.NATPortMap)
+	}
+
+	host := p2pbhost.New(network, hostOpts...)
 
 	return host, nil
 }

core/mock/mock.go

@@ -34,7 +34,7 @@ func NewMockNode() (*core.IpfsNode, error) {
 }
 
 func MockHostOption(mn mocknet.Mocknet) core.HostOption {
-	return func(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, _ smux.Transport, _ ipnet.Protector) (host.Host, error) {
+	return func(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, _ smux.Transport, _ ipnet.Protector, _ *core.ConstructPeerHostOpts) (host.Host, error) {
 		return mn.AddPeerWithPeerstore(id, ps)
 	}
 }

repo/config/config.go

@@ -34,6 +34,20 @@ type Config struct {
 	Experimental Experiments
 }
 
+// NewDefaultConfig creates any new Config with all the values set the
+// default
+func NewDefaultConfig() *Config {
+	conf := &Config{}
+	conf.SetDefaults()
+	return conf
+}
+
+func (conf *Config) SetDefaults() {
+	// sets defaults other that are not the go-standard of
+	// false, 0, or the empty string
+	conf.Swarm.NatPortMap = true
+}
+
 const (
 	// DefaultPathName is the default config dir name
 	DefaultPathName = ".ipfs"
@@ -95,11 +109,11 @@ func FromMap(v map[string]interface{}) (*Config, error) {
 	if err := json.NewEncoder(buf).Encode(v); err != nil {
 		return nil, err
 	}
-	var conf Config
-	if err := json.NewDecoder(buf).Decode(&conf); err != nil {
+	conf := NewDefaultConfig()
+	if err := json.NewDecoder(buf).Decode(conf); err != nil {
 		return nil, fmt.Errorf("Failure to decode config: %s", err)
 	}
-	return &conf, nil
+	return conf, nil
 }
 
 func ToMap(conf *Config) (map[string]interface{}, error) {

repo/config/init.go

@@ -72,6 +72,7 @@ func Init(out io.Writer, nBitsForKeypair int) (*Config, error) {
 			Interval: "12h",
 		},
 	}
+	conf.SetDefaults()
 
 	return conf, nil
 }

repo/config/swarm.go

@@ -3,4 +3,5 @@ package config
 type SwarmConfig struct {
 	AddrFilters             []string
 	DisableBandwidthMetrics bool
+	NatPortMap              bool // default: true
 }

repo/fsrepo/serialize/serialize.go

@@ -63,11 +63,11 @@ func Load(filename string) (*config.Config, error) {
 		return nil, errors.New("ipfs not initialized, please run 'ipfs init'")
 	}
 
-	var cfg config.Config
-	err := ReadConfigFile(filename, &cfg)
+	cfg := config.NewDefaultConfig()
+	err := ReadConfigFile(filename, cfg)
 	if err != nil {
 		return nil, err
 	}
 
-	return &cfg, err
+	return cfg, err
 }

test/sharness/t0023-config-default.sh

@@ -0,0 +1,51 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Christian Couder
+# MIT Licensed; see the LICENSE file in this repository.
+#
+
+test_description="Test init command with default config"
+
+. lib/test-lib.sh
+
+test_init_ipfs
+
+test_expect_success "Swarm.NatPortMap set to true in inital config" '
+	echo "true" > expected &&
+	ipfs config Swarm.NatPortMap > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success "Swarm.NatPortMap has line in config file" '
+	grep -q NatPortMap "$IPFS_PATH"/config
+'
+
+test_expect_success "remove Swarm.NatPortMap from config file" '
+	sed -i "s/NatPortMap/XxxYyyyZzz/" "$IPFS_PATH"/config
+'
+
+test_expect_success "load config file by replacing an unrelated key" '
+	# Note: this is relying on an implementation detail where setting a
+	#   config field loads the json blob into the config struct, causing
+	#   defaults to be properly set. Simply "reading" the config would read
+	#   the json directly and not cause defaults to be reset
+	ipfs config --json Swarm.DisableBandwidthMetrics false
+'
+
+test_expect_success "Swarm.NatPortMap set to true in new config" '
+	echo "true" > expected &&
+	ipfs config Swarm.NatPortMap > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success "reset Swarm group to default values" '
+	ipfs config --json Swarm {}
+'
+
+test_expect_success "Swarm.NatPortMap still set to true in reset config" '
+	echo "true" > expected &&
+	ipfs config Swarm.NatPortMap > actual &&
+	test_cmp expected actual
+'
+
+test_done