From 1341f0eee2dfec8f10fa53acac16a3a8a603894a Mon Sep 17 00:00:00 2001 From: Will Scott Date: Thu, 12 Mar 2020 08:39:56 -0700 Subject: [PATCH 1/2] change autonat interface to use functional options --- autonat.go | 57 ++++++++++++++++++++++--------------------------- autonat_test.go | 9 +------- client.go | 12 +---------- interface.go | 32 +++++++++++++++++++++++++++ notify.go | 4 ---- options.go | 57 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+), 55 deletions(-) create mode 100644 interface.go create mode 100644 options.go diff --git a/autonat.go b/autonat.go index 951d12c..a159f9b 100644 --- a/autonat.go +++ b/autonat.go @@ -17,28 +17,12 @@ import ( manet "github.com/multiformats/go-multiaddr-net" ) -var ( - AutoNATBootDelay = 15 * time.Second - AutoNATRetryInterval = 90 * time.Second - AutoNATRefreshInterval = 15 * time.Minute - AutoNATRequestTimeout = 30 * time.Second -) - -// AutoNAT is the interface for ambient NAT autodiscovery -type AutoNAT interface { - // Status returns the current NAT status - Status() network.Reachability - // PublicAddr returns the public dial address when NAT status is public and an - // error otherwise - PublicAddr() (ma.Multiaddr, error) -} - // AmbientAutoNAT is the implementation of ambient NAT autodiscovery type AmbientAutoNAT struct { ctx context.Context host host.Host - getAddrs GetAddrs + *config inboundConn chan network.Conn observations chan autoNATResult @@ -63,21 +47,30 @@ type autoNATResult struct { address ma.Multiaddr } -// NewAutoNAT creates a new ambient NAT autodiscovery instance attached to a host -// If getAddrs is nil, h.Addrs will be used -func NewAutoNAT(ctx context.Context, h host.Host, getAddrs GetAddrs) AutoNAT { - if getAddrs == nil { - getAddrs = h.Addrs +// New creates a new NAT autodiscovery system attached to a host +func New(ctx context.Context, h host.Host, options ...Option) (AutoNAT, error) { + conf := new(config) + + if err := defaults(conf); err != nil { + return nil, err + } + if conf.getAddressFunc == nil { + conf.getAddressFunc = h.Addrs } - subAddrUpdated, _ := h.EventBus().Subscribe(new(event.EvtLocalAddressesUpdated)) + for _, o := range options { + if err := o(conf); err != nil { + return nil, err + } + } + subAddrUpdated, _ := h.EventBus().Subscribe(new(event.EvtLocalAddressesUpdated)) emitReachabilityChanged, _ := h.EventBus().Emitter(new(event.EvtLocalReachabilityChanged), eventbus.Stateful) as := &AmbientAutoNAT{ ctx: ctx, host: h, - getAddrs: getAddrs, + config: conf, inboundConn: make(chan network.Conn, 5), observations: make(chan autoNATResult, 1), @@ -90,7 +83,7 @@ func NewAutoNAT(ctx context.Context, h host.Host, getAddrs GetAddrs) AutoNAT { h.Network().Notify(as) go as.background() - return as + return as, nil } // Status returns the AutoNAT observed reachability status. @@ -127,7 +120,7 @@ func ipInList(candidate ma.Multiaddr, list []ma.Multiaddr) bool { func (as *AmbientAutoNAT) background() { // wait a bit for the node to come online and establish some connections // before starting autodetection - delay := AutoNATBootDelay + delay := as.config.bootDelay var lastAddrUpdated time.Time addrUpdatedChan := as.subAddrUpdated.Out() @@ -192,11 +185,11 @@ func (as *AmbientAutoNAT) scheduleProbe() time.Duration { nextProbe := fixedNow if !as.lastProbe.IsZero() { - untilNext := AutoNATRefreshInterval + untilNext := as.config.refreshInterval if currentStatus.Reachability == network.ReachabilityUnknown { - untilNext = AutoNATRetryInterval + untilNext = as.config.retryInterval } else if as.confidence < 3 { - untilNext = AutoNATRetryInterval + untilNext = as.config.retryInterval } else if currentStatus.Reachability == network.ReachabilityPublic && as.lastInbound.After(as.lastProbe) { untilNext *= 2 } @@ -204,7 +197,7 @@ func (as *AmbientAutoNAT) scheduleProbe() time.Duration { } if fixedNow.After(nextProbe) || fixedNow == nextProbe { go as.probeNextPeer() - return AutoNATRetryInterval + return as.config.retryInterval } return nextProbe.Sub(fixedNow) } @@ -265,8 +258,8 @@ func (as *AmbientAutoNAT) recordObservation(observation autoNATResult) { } func (as *AmbientAutoNAT) probe(pi *peer.AddrInfo) { - cli := NewAutoNATClient(as.host, as.getAddrs) - ctx, cancel := context.WithTimeout(as.ctx, AutoNATRequestTimeout) + cli := NewAutoNATClient(as.host, as.config.getAddressFunc) + ctx, cancel := context.WithTimeout(as.ctx, as.config.requestTimeout) defer cancel() a, err := cli.DialBack(ctx, pi.ID) diff --git a/autonat_test.go b/autonat_test.go index ddf6d5c..0304cf3 100644 --- a/autonat_test.go +++ b/autonat_test.go @@ -17,13 +17,6 @@ import ( ma "github.com/multiformats/go-multiaddr" ) -func init() { - AutoNATBootDelay = 100 * time.Millisecond - AutoNATRefreshInterval = 1 * time.Second - AutoNATRetryInterval = 1 * time.Second - AutoNATIdentifyDelay = 100 * time.Millisecond -} - // these are mock service implementations for testing func makeAutoNATServicePrivate(ctx context.Context, t *testing.T) host.Host { h := bhost.NewBlankHost(swarmt.GenSwarm(t, ctx)) @@ -75,7 +68,7 @@ func makeAutoNAT(ctx context.Context, t *testing.T, ash host.Host) (host.Host, A h := bhost.NewBlankHost(swarmt.GenSwarm(t, ctx)) h.Peerstore().AddAddrs(ash.ID(), ash.Addrs(), time.Minute) h.Peerstore().AddProtocols(ash.ID(), AutoNATProto) - a := NewAutoNAT(ctx, h, nil) + a, _ := New(ctx, h, WithSchedule(100*time.Millisecond, time.Second), WithoutStartupDelay()) return h, a } diff --git a/client.go b/client.go index 0d8549b..e2acab1 100644 --- a/client.go +++ b/client.go @@ -14,25 +14,15 @@ import ( ma "github.com/multiformats/go-multiaddr" ) -// AutoNATClient is a stateless client interface to AutoNAT peers -type AutoNATClient interface { - // DialBack requests from a peer providing AutoNAT services to test dial back - // and report the address on a successful connection. - DialBack(ctx context.Context, p peer.ID) (ma.Multiaddr, error) -} - // AutoNATError is the class of errors signalled by AutoNAT services type AutoNATError struct { Status pb.Message_ResponseStatus Text string } -// GetAddrs is a function that returns the addresses to dial back -type GetAddrs func() []ma.Multiaddr - // NewAutoNATClient creates a fresh instance of an AutoNATClient // If getAddrs is nil, h.Addrs will be used -func NewAutoNATClient(h host.Host, getAddrs GetAddrs) AutoNATClient { +func NewAutoNATClient(h host.Host, getAddrs GetAddrs) Client { if getAddrs == nil { getAddrs = h.Addrs } diff --git a/interface.go b/interface.go new file mode 100644 index 0000000..af010b9 --- /dev/null +++ b/interface.go @@ -0,0 +1,32 @@ +package autonat + +import ( + "context" + + "github.com/libp2p/go-libp2p-core/network" + "github.com/libp2p/go-libp2p-core/peer" + + ma "github.com/multiformats/go-multiaddr" +) + +// AutoNAT is the interface for NAT autodiscovery +type AutoNAT interface { + // Status returns the current NAT status + Status() network.Reachability + // PublicAddr returns the public dial address when NAT status is public and an + // error otherwise + PublicAddr() (ma.Multiaddr, error) +} + +// Client is a stateless client interface to AutoNAT peers +type Client interface { + // DialBack requests from a peer providing AutoNAT services to test dial back + // and report the address on a successful connection. + DialBack(ctx context.Context, p peer.ID) (ma.Multiaddr, error) +} + +// GetAddrs is a function returning the candidate addresses for the local host. +type GetAddrs func() []ma.Multiaddr + +// Option is an Autonat option for configuration +type Option func(*config) error diff --git a/notify.go b/notify.go index ed8d870..a444df6 100644 --- a/notify.go +++ b/notify.go @@ -1,8 +1,6 @@ package autonat import ( - "time" - "github.com/libp2p/go-libp2p-core/network" ma "github.com/multiformats/go-multiaddr" @@ -11,8 +9,6 @@ import ( var _ network.Notifiee = (*AmbientAutoNAT)(nil) -var AutoNATIdentifyDelay = 5 * time.Second - func (as *AmbientAutoNAT) Listen(net network.Network, a ma.Multiaddr) {} func (as *AmbientAutoNAT) ListenClose(net network.Network, a ma.Multiaddr) {} func (as *AmbientAutoNAT) OpenedStream(net network.Network, s network.Stream) {} diff --git a/options.go b/options.go new file mode 100644 index 0000000..4d75349 --- /dev/null +++ b/options.go @@ -0,0 +1,57 @@ +package autonat + +import ( + "time" +) + +// config holds configurable options for the autonat subsystem. +type config struct { + getAddressFunc GetAddrs + bootDelay time.Duration + retryInterval time.Duration + refreshInterval time.Duration + requestTimeout time.Duration +} + +var defaults = func(c *config) error { + c.bootDelay = 15 * time.Second + c.retryInterval = 90 * time.Second + c.refreshInterval = 15 * time.Minute + c.requestTimeout = 30 * time.Second + + return nil +} + +// WithAddresses allows overriding which Addresses the AutoNAT client beliieves +// are "its own". Useful for testing, or for more exotic port-forwarding +// scenarios where the host may be listening on different ports than it wants +// to externally advertise or verify connectability on. +func WithAddresses(addrFunc GetAddrs) Option { + return func(c *config) error { + c.getAddressFunc = addrFunc + return nil + } +} + +// WithSchedule configures how agressively probes will be made to verify the +// address of the host. retryInterval indicates how often probes should be made +// when the host lacks confident about its address, while refresh interval +// is the schedule of periodic probes when the host believes it knows its +// steady-state reachability. +func WithSchedule(retryInterval, refreshInterval time.Duration) Option { + return func(c *config) error { + c.retryInterval = retryInterval + c.refreshInterval = refreshInterval + return nil + } +} + +// WithoutStartupDelay removes the initial delay the NAT subsystem typically +// uses as a buffer for ensuring that connectivity and guesses as to the hosts +// local interfaces have settled down during startup. +func WithoutStartupDelay() Option { + return func(c *config) error { + c.bootDelay = 1 + return nil + } +} From 62f42d93464d523fccc59bb7740975301d7bc1c7 Mon Sep 17 00:00:00 2001 From: Will Scott Date: Thu, 12 Mar 2020 11:45:22 -0700 Subject: [PATCH 2/2] remove a foot-gun --- options.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/options.go b/options.go index 4d75349..6987d3a 100644 --- a/options.go +++ b/options.go @@ -1,6 +1,7 @@ package autonat import ( + "errors" "time" ) @@ -28,6 +29,9 @@ var defaults = func(c *config) error { // to externally advertise or verify connectability on. func WithAddresses(addrFunc GetAddrs) Option { return func(c *config) error { + if addrFunc == nil { + return errors.New("invalid address function supplied") + } c.getAddressFunc = addrFunc return nil }