cleanup: netx does not use netxlite legacy names (#801)

This diff refactors netx and netxlite to ensure we're not using
netxlite legacy names inside of netx.

To this end, we're cheating a bit. We're exposing a new factory to
get an unwrapped stdlib resolver rather than defining a legacy name
to export the private name of the same factory.

This is actually a fine place to stop, for now, the next and
netxlite refactoring at https://github.com/ooni/probe/issues/2121.
This commit is contained in:
Simone Basso 2022-06-06 14:46:44 +02:00 committed by GitHub
parent 64bffbd941
commit 2502a237fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 34 additions and 43 deletions

View File

@ -54,7 +54,7 @@ func TestWorkingAsIntended(t *testing.T) {
Client: http.DefaultClient, Client: http.DefaultClient,
Dialer: netxlite.NewDialerWithStdlibResolver(model.DiscardLogger), Dialer: netxlite.NewDialerWithStdlibResolver(model.DiscardLogger),
MaxAcceptableBody: 1 << 24, MaxAcceptableBody: 1 << 24,
Resolver: netxlite.NewResolverSystem(), Resolver: netxlite.NewUnwrappedStdlibResolver(),
} }
srv := httptest.NewServer(handler) srv := httptest.NewServer(handler)
defer srv.Close() defer srv.Close()

View File

@ -30,7 +30,7 @@ func newDialManager(ndt7URL string, logger model.Logger, userAgent string) dialM
} }
func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*websocket.Conn, error) { func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*websocket.Conn, error) {
reso := netxlite.NewResolverStdlib(mgr.logger) reso := netxlite.NewStdlibResolver(mgr.logger)
dlr := netxlite.NewDialerWithResolver(mgr.logger, reso) dlr := netxlite.NewDialerWithResolver(mgr.logger, reso)
dlr = bytecounter.WrapWithContextAwareDialer(dlr) dlr = bytecounter.WrapWithContextAwareDialer(dlr)
// Implements shaping if the user builds using `-tags shaping` // Implements shaping if the user builds using `-tags shaping`

View File

@ -114,7 +114,7 @@ func NewTask(config Config) *Task {
config.UserAgent = fmt.Sprintf("ooniprobe-engine/%s", version.Version) config.UserAgent = fmt.Sprintf("ooniprobe-engine/%s", version.Version)
} }
if config.Resolver == nil { if config.Resolver == nil {
config.Resolver = netxlite.NewResolverStdlib(config.Logger) config.Resolver = netxlite.NewStdlibResolver(config.Logger)
} }
return &Task{ return &Task{
countryLookupper: mmdbLookupper{}, countryLookupper: mmdbLookupper{},

View File

@ -14,7 +14,7 @@ import (
func TestIPLookupGood(t *testing.T) { func TestIPLookupGood(t *testing.T) {
ip, err := (ipLookupClient{ ip, err := (ipLookupClient{
Logger: log.Log, Logger: log.Log,
Resolver: netxlite.NewResolverStdlib(model.DiscardLogger), Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
UserAgent: "ooniprobe-engine/0.1.0", UserAgent: "ooniprobe-engine/0.1.0",
}).LookupProbeIP(context.Background()) }).LookupProbeIP(context.Background())
if err != nil { if err != nil {
@ -30,7 +30,7 @@ func TestIPLookupAllFailed(t *testing.T) {
cancel() // immediately cancel to cause Do() to fail cancel() // immediately cancel to cause Do() to fail
ip, err := (ipLookupClient{ ip, err := (ipLookupClient{
Logger: log.Log, Logger: log.Log,
Resolver: netxlite.NewResolverStdlib(model.DiscardLogger), Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
UserAgent: "ooniprobe-engine/0.1.0", UserAgent: "ooniprobe-engine/0.1.0",
}).LookupProbeIP(ctx) }).LookupProbeIP(ctx)
if !errors.Is(err, context.Canceled) { if !errors.Is(err, context.Canceled) {
@ -45,7 +45,7 @@ func TestIPLookupInvalidIP(t *testing.T) {
ctx := context.Background() ctx := context.Background()
ip, err := (ipLookupClient{ ip, err := (ipLookupClient{
Logger: log.Log, Logger: log.Log,
Resolver: netxlite.NewResolverStdlib(model.DiscardLogger), Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
UserAgent: "ooniprobe-engine/0.1.0", UserAgent: "ooniprobe-engine/0.1.0",
}).doWithCustomFunc(ctx, invalidIPLookup) }).doWithCustomFunc(ctx, invalidIPLookup)
if !errors.Is(err, ErrInvalidIPAddress) { if !errors.Is(err, ErrInvalidIPAddress) {

View File

@ -65,7 +65,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
} }
switch resolverURL.Scheme { switch resolverURL.Scheme {
case "system": case "system":
return netxlite.NewResolverSystem(), nil return netxlite.NewUnwrappedStdlibResolver(), nil
case "https": case "https":
config.TLSConfig.NextProtos = []string{"h2", "http/1.1"} config.TLSConfig.NextProtos = []string{"h2", "http/1.1"}
httpClient := &http.Client{Transport: NewHTTPTransport(config)} httpClient := &http.Client{Transport: NewHTTPTransport(config)}

View File

@ -12,7 +12,7 @@ import (
// NewResolver creates a new resolver from the specified config. // NewResolver creates a new resolver from the specified config.
func NewResolver(config Config) model.Resolver { func NewResolver(config Config) model.Resolver {
if config.BaseResolver == nil { if config.BaseResolver == nil {
config.BaseResolver = netxlite.NewResolverSystem() config.BaseResolver = netxlite.NewUnwrappedStdlibResolver()
} }
r := netxlite.WrapResolver( r := netxlite.WrapResolver(
model.ValidLoggerOrDefault(config.Logger), model.ValidLoggerOrDefault(config.Logger),

View File

@ -28,7 +28,7 @@ func WrapResolver(begin time.Time, db WritableDB, r model.Resolver) model.Resolv
// NewResolverSystem creates a system resolver and then wraps // NewResolverSystem creates a system resolver and then wraps
// it using the WrapResolver function. // it using the WrapResolver function.
func (mx *Measurer) NewResolverSystem(db WritableDB, logger model.Logger) model.Resolver { func (mx *Measurer) NewResolverSystem(db WritableDB, logger model.Logger) model.Resolver {
return mx.WrapResolver(db, netxlite.NewResolverStdlib(logger)) return mx.WrapResolver(db, netxlite.NewStdlibResolver(logger))
} }
// NewResolverUDP is a convenience factory for creating a Resolver // NewResolverUDP is a convenience factory for creating a Resolver

View File

@ -54,7 +54,7 @@ func NewTracingHTTPTransport(logger model.Logger, begin time.Time, db WritableDB
func NewTracingHTTPTransportWithDefaultSettings( func NewTracingHTTPTransportWithDefaultSettings(
begin time.Time, logger model.Logger, db WritableDB) *HTTPTransportDB { begin time.Time, logger model.Logger, db WritableDB) *HTTPTransportDB {
return NewTracingHTTPTransport(logger, begin, db, return NewTracingHTTPTransport(logger, begin, db,
netxlite.NewResolverStdlib(logger), netxlite.NewStdlibResolver(logger),
netxlite.NewDialerWithoutResolver(logger), netxlite.NewDialerWithoutResolver(logger),
netxlite.NewTLSHandshakerStdlib(logger), netxlite.NewTLSHandshakerStdlib(logger),
DefaultHTTPMaxBodySnapshotSize) DefaultHTTPMaxBodySnapshotSize)

View File

@ -15,10 +15,10 @@ import (
) )
// NewDialerWithStdlibResolver is equivalent to creating a system resolver // NewDialerWithStdlibResolver is equivalent to creating a system resolver
// using NewResolverStdlib and then a dialer using NewDialerWithResolver where // using NewStdlibResolver and then a dialer using NewDialerWithResolver where
// the resolver argument is the previously created resolver. // the resolver argument is the previously created resolver.
func NewDialerWithStdlibResolver(dl model.DebugLogger) model.Dialer { func NewDialerWithStdlibResolver(dl model.DebugLogger) model.Dialer {
reso := NewResolverStdlib(dl) reso := NewStdlibResolver(dl)
return NewDialerWithResolver(dl, reso) return NewDialerWithResolver(dl, reso)
} }

View File

@ -141,7 +141,7 @@ func TestDialerResolver(t *testing.T) {
t.Run("fails without a port", func(t *testing.T) { t.Run("fails without a port", func(t *testing.T) {
d := &dialerResolver{ d := &dialerResolver{
Dialer: &DialerSystem{}, Dialer: &DialerSystem{},
Resolver: newResolverSystem(), Resolver: NewUnwrappedStdlibResolver(),
} }
const missingPort = "ooni.nu" const missingPort = "ooni.nu"
conn, err := d.DialContext(context.Background(), "tcp", missingPort) conn, err := d.DialContext(context.Background(), "tcp", missingPort)

View File

@ -323,7 +323,7 @@ func (c *httpTLSConnWithReadTimeout) Read(b []byte) (int, error) {
// This factory and NewHTTPTransport are the recommended // This factory and NewHTTPTransport are the recommended
// ways of creating a new HTTPTransport. // ways of creating a new HTTPTransport.
func NewHTTPTransportStdlib(logger model.DebugLogger) model.HTTPTransport { func NewHTTPTransportStdlib(logger model.DebugLogger) model.HTTPTransport {
dialer := NewDialerWithResolver(logger, NewResolverStdlib(logger)) dialer := NewDialerWithResolver(logger, NewStdlibResolver(logger))
tlsDialer := NewTLSDialer(dialer, NewTLSHandshakerStdlib(logger)) tlsDialer := NewTLSDialer(dialer, NewTLSHandshakerStdlib(logger))
return NewHTTPTransport(logger, dialer, tlsDialer) return NewHTTPTransport(logger, dialer, tlsDialer)
} }

View File

@ -236,7 +236,7 @@ func TestNewHTTPTransport(t *testing.T) {
called.Add(1) called.Add(1)
}, },
}, },
Resolver: NewResolverStdlib(log.Log), Resolver: NewStdlibResolver(log.Log),
} }
td := NewTLSDialer(d, NewTLSHandshakerStdlib(log.Log)) td := NewTLSDialer(d, NewTLSHandshakerStdlib(log.Log))
txp := NewHTTPTransport(log.Log, d, td) txp := NewHTTPTransport(log.Log, d, td)

View File

@ -41,7 +41,7 @@ func TestMeasureWithSystemResolver(t *testing.T) {
// //
t.Run("on success", func(t *testing.T) { t.Run("on success", func(t *testing.T) {
r := netxlite.NewResolverStdlib(log.Log) r := netxlite.NewStdlibResolver(log.Log)
defer r.CloseIdleConnections() defer r.CloseIdleConnections()
ctx := context.Background() ctx := context.Background()
addrs, err := r.LookupHost(ctx, "dns.google.com") addrs, err := r.LookupHost(ctx, "dns.google.com")
@ -54,7 +54,7 @@ func TestMeasureWithSystemResolver(t *testing.T) {
}) })
t.Run("for nxdomain", func(t *testing.T) { t.Run("for nxdomain", func(t *testing.T) {
r := netxlite.NewResolverStdlib(log.Log) r := netxlite.NewStdlibResolver(log.Log)
defer r.CloseIdleConnections() defer r.CloseIdleConnections()
ctx := context.Background() ctx := context.Background()
addrs, err := r.LookupHost(ctx, "antani.ooni.org") addrs, err := r.LookupHost(ctx, "antani.ooni.org")
@ -67,7 +67,7 @@ func TestMeasureWithSystemResolver(t *testing.T) {
}) })
t.Run("for timeout", func(t *testing.T) { t.Run("for timeout", func(t *testing.T) {
r := netxlite.NewResolverStdlib(log.Log) r := netxlite.NewStdlibResolver(log.Log)
defer r.CloseIdleConnections() defer r.CloseIdleConnections()
const timeout = time.Nanosecond const timeout = time.Nanosecond
ctx, cancel := context.WithTimeout(context.Background(), timeout) ctx, cancel := context.WithTimeout(context.Background(), timeout)
@ -472,7 +472,7 @@ func TestHTTPTransport(t *testing.T) {
} }
t.Run("works as intended", func(t *testing.T) { t.Run("works as intended", func(t *testing.T) {
d := netxlite.NewDialerWithResolver(log.Log, netxlite.NewResolverStdlib(log.Log)) d := netxlite.NewDialerWithResolver(log.Log, netxlite.NewStdlibResolver(log.Log))
td := netxlite.NewTLSDialer(d, netxlite.NewTLSHandshakerStdlib(log.Log)) td := netxlite.NewTLSDialer(d, netxlite.NewTLSHandshakerStdlib(log.Log))
txp := netxlite.NewHTTPTransport(log.Log, d, td) txp := netxlite.NewHTTPTransport(log.Log, d, td)
client := &http.Client{Transport: txp} client := &http.Client{Transport: txp}
@ -523,7 +523,7 @@ func TestHTTP3Transport(t *testing.T) {
d := netxlite.NewQUICDialerWithResolver( d := netxlite.NewQUICDialerWithResolver(
netxlite.NewQUICListener(), netxlite.NewQUICListener(),
log.Log, log.Log,
netxlite.NewResolverStdlib(log.Log), netxlite.NewStdlibResolver(log.Log),
) )
txp := netxlite.NewHTTP3Transport(log.Log, d, &tls.Config{}) txp := netxlite.NewHTTP3Transport(log.Log, d, &tls.Config{})
client := &http.Client{Transport: txp} client := &http.Client{Transport: txp}

View File

@ -1,12 +0,0 @@
package netxlite
//
// Legacy code
//
// These vars export internal names to legacy ooni/probe-cli code.
//
// Deprecated: do not use these names in new code.
var (
NewResolverSystem = newResolverSystem
)

View File

@ -486,7 +486,7 @@ func TestQUICDialerResolver(t *testing.T) {
t.Run("with missing port", func(t *testing.T) { t.Run("with missing port", func(t *testing.T) {
tlsConfig := &tls.Config{} tlsConfig := &tls.Config{}
dialer := &quicDialerResolver{ dialer := &quicDialerResolver{
Resolver: NewResolverStdlib(log.Log), Resolver: NewStdlibResolver(log.Log),
Dialer: &quicDialerQUICGo{}} Dialer: &quicDialerQUICGo{}}
qconn, err := dialer.DialContext( qconn, err := dialer.DialContext(
context.Background(), "udp", "www.google.com", context.Background(), "udp", "www.google.com",
@ -523,7 +523,7 @@ func TestQUICDialerResolver(t *testing.T) {
// to establish a connection leads to a failure // to establish a connection leads to a failure
tlsConf := &tls.Config{} tlsConf := &tls.Config{}
dialer := &quicDialerResolver{ dialer := &quicDialerResolver{
Resolver: NewResolverStdlib(log.Log), Resolver: NewStdlibResolver(log.Log),
Dialer: &quicDialerQUICGo{ Dialer: &quicDialerQUICGo{
QUICListener: &quicListenerStdlib{}, QUICListener: &quicListenerStdlib{},
}} }}
@ -546,7 +546,7 @@ func TestQUICDialerResolver(t *testing.T) {
var gotTLSConfig *tls.Config var gotTLSConfig *tls.Config
tlsConfig := &tls.Config{} tlsConfig := &tls.Config{}
dialer := &quicDialerResolver{ dialer := &quicDialerResolver{
Resolver: NewResolverStdlib(log.Log), Resolver: NewStdlibResolver(log.Log),
Dialer: &mocks.QUICDialer{ Dialer: &mocks.QUICDialer{
MockDialContext: func(ctx context.Context, network, address string, MockDialContext: func(ctx context.Context, network, address string,
tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) { tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) {
@ -574,7 +574,7 @@ func TestQUICDialerResolver(t *testing.T) {
t.Run("on success", func(t *testing.T) { t.Run("on success", func(t *testing.T) {
expectedQConn := &mocks.QUICEarlyConnection{} expectedQConn := &mocks.QUICEarlyConnection{}
dialer := &quicDialerResolver{ dialer := &quicDialerResolver{
Resolver: NewResolverStdlib(log.Log), Resolver: NewStdlibResolver(log.Log),
Dialer: &mocks.QUICDialer{ Dialer: &mocks.QUICDialer{
MockDialContext: func(ctx context.Context, network, address string, MockDialContext: func(ctx context.Context, network, address string,
tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) { tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) {

View File

@ -23,12 +23,12 @@ import (
// but you are using the "system" resolver instead. // but you are using the "system" resolver instead.
var ErrNoDNSTransport = errors.New("operation requires a DNS transport") var ErrNoDNSTransport = errors.New("operation requires a DNS transport")
// NewResolverStdlib creates a new Resolver by combining WrapResolver // NewStdlibResolver creates a new Resolver by combining WrapResolver
// with an internal "system" resolver type. The list of optional wrappers // with an internal "system" resolver type. The list of optional wrappers
// allow to wrap the underlying getaddrinfo transport. Any nil wrapper // allow to wrap the underlying getaddrinfo transport. Any nil wrapper
// will be silently ignored by the code that performs the wrapping. // will be silently ignored by the code that performs the wrapping.
func NewResolverStdlib(logger model.DebugLogger, wrappers ...model.DNSTransportWrapper) model.Resolver { func NewStdlibResolver(logger model.DebugLogger, wrappers ...model.DNSTransportWrapper) model.Resolver {
return WrapResolver(logger, newResolverSystem(wrappers...)) return WrapResolver(logger, NewUnwrappedStdlibResolver(wrappers...))
} }
// NewParallelDNSOverHTTPSResolver creates a new DNS over HTTPS resolver // NewParallelDNSOverHTTPSResolver creates a new DNS over HTTPS resolver
@ -40,7 +40,10 @@ func NewParallelDNSOverHTTPSResolver(logger model.DebugLogger, URL string) model
return WrapResolver(logger, NewUnwrappedParallelResolver(txp)) return WrapResolver(logger, NewUnwrappedParallelResolver(txp))
} }
func newResolverSystem(wrappers ...model.DNSTransportWrapper) *resolverSystem { // NewUnwrappedStdlibResolver returns a new, unwrapped resolver using the standard
// library (i.e., getaddrinfo if possible and &net.Resolver{} otherwise). As the name
// implies, this function returns an unwrapped resolver.
func NewUnwrappedStdlibResolver(wrappers ...model.DNSTransportWrapper) model.Resolver {
return &resolverSystem{ return &resolverSystem{
t: WrapDNSTransport(&dnsOverGetaddrinfoTransport{}, wrappers...), t: WrapDNSTransport(&dnsOverGetaddrinfoTransport{}, wrappers...),
} }

View File

@ -29,7 +29,7 @@ func typecheckForSystemResolver(t *testing.T, resolver model.Resolver, logger mo
} }
func TestNewResolverSystem(t *testing.T) { func TestNewResolverSystem(t *testing.T) {
resolver := NewResolverStdlib(model.DiscardLogger) resolver := NewStdlibResolver(model.DiscardLogger)
typecheckForSystemResolver(t, resolver, model.DiscardLogger) typecheckForSystemResolver(t, resolver, model.DiscardLogger)
} }

View File

@ -51,7 +51,7 @@ The returned resolver implements an interface that is very
close to the API of the `net.Resolver` struct. close to the API of the `net.Resolver` struct.
```Go ```Go
reso := netxlite.NewResolverStdlib(log.Log) reso := netxlite.NewStdlibResolver(log.Log)
``` ```
We call `LookupHost` to map the hostname to IP addrs. The returned We call `LookupHost` to map the hostname to IP addrs. The returned

View File

@ -52,7 +52,7 @@ func main() {
// close to the API of the `net.Resolver` struct. // close to the API of the `net.Resolver` struct.
// //
// ```Go // ```Go
reso := netxlite.NewResolverStdlib(log.Log) reso := netxlite.NewStdlibResolver(log.Log)
// ``` // ```
// //
// We call `LookupHost` to map the hostname to IP addrs. The returned // We call `LookupHost` to map the hostname to IP addrs. The returned