refactor: clean up outline-connectivity

CONTRIBUTING.md

@@ -43,23 +43,23 @@ In Go you can compile for other target operating system and architecture by spec
 
 MacOS example:
 ```
-% GOOS=darwin go build -C x -o ./bin/ ./outline-connectivity 
-% file ./x/bin/outline-connectivity 
-./x/bin/outline-connectivity: Mach-O 64-bit executable x86_64
+% GOOS=darwin go build -C x -o ./bin/ ./examples/test-connectivity 
+% file ./x/bin/test-connectivity 
+./x/bin/test-connectivity: Mach-O 64-bit executable x86_64
 ```
 
 Linux example:
 ```
-% GOOS=linux go build -C x -o ./bin/ ./outline-connectivity 
-% file ./x/bin/outline-connectivity                      
-./x/bin/outline-connectivity: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, Go BuildID=n0WfUGLum4Y6OpYxZYuz/lbtEdv_kvyUCd3V_qOqb/CC_6GAQqdy_ebeYTdn99/Tk_G3WpBWi8vxqmIlIuU, with debug_info, not stripped
+% GOOS=linux go build -C x -o ./bin/ ./examples/test-connectivity 
+% file ./x/bin/test-connectivity                      
+./x/bin/test-connectivity: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, Go BuildID=n0WfUGLum4Y6OpYxZYuz/lbtEdv_kvyUCd3V_qOqb/CC_6GAQqdy_ebeYTdn99/Tk_G3WpBWi8vxqmIlIuU, with debug_info, not stripped
 ```
 
 Windows example:
 ```
-% GOOS=windows go build -C x -o ./bin/ ./outline-connectivity 
-% file ./x/bin/outline-connectivity.exe 
-./x/bin/outline-connectivity.exe: PE32+ executable (console) x86-64 (stripped to external PDB), for MS Windows
+% GOOS=windows go build -C x -o ./bin/ ./examples/test-connectivity 
+% file ./x/bin/test-connectivity.exe 
+./x/bin/test-connectivity.exe: PE32+ executable (console) x86-64 (stripped to external PDB), for MS Windows
 ```
 </details>
 
@@ -103,7 +103,7 @@ podman machine stop
 
 The easiest way is to run a binary is to use the [`go run` command](https://pkg.go.dev/cmd/go#hdr-Compile_and_run_Go_program) directly with the `-exec` flag and our convenience tool `run_on_podman.sh`:
 ```sh
-GOOS=linux go run -C x -exec "$(pwd)/run_on_podman.sh" ./outline-connectivity
+GOOS=linux go run -C x -exec "$(pwd)/run_on_podman.sh" ./examples/test-connectivity
 ```
 
 It also works with the [`go test` command](https://pkg.go.dev/cmd/go#hdr-Test_packages):
@@ -121,8 +121,8 @@ podman run --arch $(uname -m) --rm -it -v "${bin}":/outline/bin gcr.io/distroles
 
 You can also use `podman run` directly to run a pre-built binary:
 ```
-% podman run --rm -it -v ./x/bin:/outline gcr.io/distroless/static-debian11 /outline/outline-connectivity
-Usage of /outline/outline-connectivity:
+% podman run --rm -it -v ./x/bin:/outline gcr.io/distroless/static-debian11 /outline/test-connectivity
+Usage of /outline/test-connectivity:
   -domain string
         Domain name to resolve in the test (default "example.com.")
   -key string
@@ -173,7 +173,7 @@ You can pass `wine64` as the `-exec` parameter in the `go` calls.
 To build:
 
 ```sh
-GOOS=windows go run -C x -exec "wine64" ./outline-connectivity
+GOOS=windows go run -C x -exec "wine64" ./examples/test-connectivity
 ```
 
 For tests:

README.md

@@ -119,7 +119,7 @@ This launch is currently in Beta. Most of the code is not new. It's the same cod
     - [x] Connectivity Test mobile app (iOS and Android) using [Capacitor](https://capacitorjs.com/)
   - For Go apps
     - [x] Connectivity Test example [Wails](https://wails.io/) graphical app
-    - [x] Connectivity Test example command-line app ([source](./x/examples/outline-connectivity/))
+    - [x] Connectivity Test example command-line app ([source](./x/examples/test-connectivity/))
     - [x] Outline Client example command-line app ([source](./x/examples/outline-cli/))
     - [x] Page fetch example command-line app ([source](./x/examples/outline-fetch/))
     - [x] Local proxy example command-line app ([source](./x/examples/http2transport/))

x/connectivity/connectivity.go

@@ -26,55 +26,75 @@ import (
 	"github.com/miekg/dns"
 )
 
-// TestError captures the observed error of the connectivity test.
-type TestError struct {
-	// Which operation in the test that failed: "dial", "write" or "read"
+// ConnectivityError captures the observed error of the connectivity test.
+type ConnectivityError struct {
+	// Which operation in the test that failed: "connect", "send" or "receive"
 	Op string
 	// The POSIX error, when available
 	PosixError string
 	// The error observed for the action
 	Err error
 }
 
-var _ error = (*TestError)(nil)
+var _ error = (*ConnectivityError)(nil)
 
-func (err *TestError) Error() string {
+func (err *ConnectivityError) Error() string {
 	return fmt.Sprintf("%v: %v", err.Op, err.Err)
 }
 
-func (err *TestError) Unwrap() error {
+func (err *ConnectivityError) Unwrap() error {
 	return err.Err
 }
 
-// TestResolverStreamConnectivity uses the given [transport.StreamEndpoint] to connect to a DNS resolver and resolve the test domain.
-// The context can be used to set a timeout or deadline, or to pass values to the dialer.
-func TestResolverStreamConnectivity(ctx context.Context, resolver transport.StreamEndpoint, testDomain string) (time.Duration, error) {
-	return testResolver(ctx, resolver.Connect, testDomain)
+// Resolver encapsulates the DNS resolution logic for connectivity tests.
+type Resolver func(context.Context) (net.Conn, error)
+
+func (r Resolver) connect(ctx context.Context) (*dns.Conn, error) {
+	conn, err := r(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return &dns.Conn{Conn: conn}, nil
 }
 
-// TestResolverPacketConnectivity uses the given [transport.PacketEndpoint] to connect to a DNS resolver and resolve the test domain.
-// The context can be used to set a timeout or deadline, or to pass values to the listener.
-func TestResolverPacketConnectivity(ctx context.Context, resolver transport.PacketEndpoint, testDomain string) (time.Duration, error) {
-	return testResolver(ctx, resolver.Connect, testDomain)
+// NewTCPResolver creates a [Resolver] to test StreamDialers.
+func NewTCPResolver(dialer transport.StreamDialer, resolverAddr string) Resolver {
+	endpoint := transport.StreamDialerEndpoint{Dialer: dialer, Address: resolverAddr}
+	return Resolver(func(ctx context.Context) (net.Conn, error) {
+		return endpoint.Connect(ctx)
+	})
+}
+
+// NewUDPResolver creates a [Resolver] to test PacketDialers.
+func NewUDPResolver(dialer transport.PacketDialer, resolverAddr string) Resolver {
+	endpoint := transport.PacketDialerEndpoint{Dialer: dialer, Address: resolverAddr}
+	return Resolver(func(ctx context.Context) (net.Conn, error) {
+		return endpoint.Connect(ctx)
+	})
 }
 
 func isTimeout(err error) bool {
 	var timeErr interface{ Timeout() bool }
 	return errors.As(err, &timeErr) && timeErr.Timeout()
 }
 
-func makeTestError(op string, err error) error {
+func makeConnectivityError(op string, err error) *ConnectivityError {
 	var code string
 	var errno syscall.Errno
 	if errors.As(err, &errno) {
 		code = errnoName(errno)
 	} else if isTimeout(err) {
 		code = "ETIMEDOUT"
 	}
-	return &TestError{Op: op, PosixError: code, Err: err}
+	return &ConnectivityError{Op: op, PosixError: code, Err: err}
 }
 
-func testResolver[C net.Conn](ctx context.Context, connect func(context.Context) (C, error), testDomain string) (time.Duration, error) {
+// TestConnectivityWithResolver tests weather we can get a response from the given [Resolver]. It can be used
+// to test connectivity of its underlying [transport.StreamDialer] or [transport.PacketDialer].
+// Invalid tests that cannot assert connectivity will return (nil, error).
+// Valid tests will return (*ConnectivityError, nil), where *ConnectivityError will be nil if there's connectivity or
+// a structure with details of the error found.
+func TestConnectivityWithResolver(ctx context.Context, resolver Resolver, testDomain string) (*ConnectivityError, error) {
 	deadline, ok := ctx.Deadline()
 	if !ok {
 		// Default deadline is 5 seconds.
@@ -84,32 +104,24 @@ func testResolver[C net.Conn](ctx context.Context, connect func(context.Context)
 		// Releases the timer.
 		defer cancel()
 	}
-	testTime := time.Now()
-	testErr := func() error {
-		conn, dialErr := connect(ctx)
-		if dialErr != nil {
-			return makeTestError("dial", dialErr)
-		}
-		defer conn.Close()
-		conn.SetDeadline(deadline)
-		dnsConn := dns.Conn{Conn: conn}
-
-		var dnsRequest dns.Msg
-		dnsRequest.SetQuestion(dns.Fqdn(testDomain), dns.TypeA)
-		writeErr := dnsConn.WriteMsg(&dnsRequest)
-		if writeErr != nil {
-			return makeTestError("write", writeErr)
-		}
-
-		_, readErr := dnsConn.ReadMsg()
-		if readErr != nil {
-			// An early close on the connection may cause a "unexpected EOF" error. That's an application-layer error,
-			// not triggered by a syscall error so we don't capture an error code.
-			// TODO: figure out how to standardize on those errors.
-			return makeTestError("read", readErr)
-		}
-		return nil
-	}()
-	duration := time.Since(testTime)
-	return duration, testErr
+
+	dnsConn, err := resolver.connect(ctx)
+	if err != nil {
+		return makeConnectivityError("connect", err), nil
+	}
+	defer dnsConn.Close()
+	dnsConn.SetDeadline(deadline)
+
+	var dnsRequest dns.Msg
+	dnsRequest.SetQuestion(dns.Fqdn(testDomain), dns.TypeA)
+	if err := dnsConn.WriteMsg(&dnsRequest); err != nil {
+		return makeConnectivityError("send", err), nil
+	}
+	if _, err := dnsConn.ReadMsg(); err != nil {
+		// An early close on the connection may cause a "unexpected EOF" error. That's an application-layer error,
+		// not triggered by a syscall error so we don't capture an error code.
+		// TODO: figure out how to standardize on those errors.
+		return makeConnectivityError("receive", err), nil
+	}
+	return nil, nil
 }

x/connectivity/connectivity_test.go

@@ -27,17 +27,18 @@ import (
 	"time"
 
 	"github.com/Jigsaw-Code/outline-sdk/transport"
-	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/net/dns/dnsmessage"
 )
 
 // StreamDialer Tests
 func TestTestResolverStreamConnectivityOk(t *testing.T) {
 	// TODO(fortuna): Run a local resolver and make test not depend on an external server.
-	resolver := &transport.TCPEndpoint{Address: "8.8.8.8:53"}
-	_, err := TestResolverStreamConnectivity(context.Background(), resolver, "example.com")
+	resolver := NewTCPResolver(&transport.TCPStreamDialer{}, "8.8.8.8:53")
+	result, err := TestConnectivityWithResolver(context.Background(), resolver, "example.com")
 	require.NoError(t, err)
+	require.Nil(t, result)
 }
 
 // TODO: Move this to the SDK.
@@ -69,23 +70,23 @@ func TestTestResolverStreamConnectivityRefused(t *testing.T) {
 	// Close right away to ensure the port is closed. The OS will likely not reuse it soon enough.
 	require.Nil(t, listener.Close())
 
-	resolver := &transport.TCPEndpoint{Address: listener.Addr().String()}
-	_, err = TestResolverStreamConnectivity(context.Background(), resolver, "anything")
-	var testErr *TestError
-	require.ErrorAs(t, err, &testErr)
-	require.Equal(t, "dial", testErr.Op)
-	require.Equal(t, "ECONNREFUSED", testErr.PosixError)
+	resolver := NewTCPResolver(&transport.TCPStreamDialer{}, listener.Addr().String())
+	result, err := TestConnectivityWithResolver(context.Background(), resolver, "anything")
+	require.NoError(t, err)
+	require.NotNil(t, result)
+	require.Equal(t, "connect", result.Op)
+	require.Equal(t, "ECONNREFUSED", result.PosixError)
 
 	var sysErr *os.SyscallError
-	require.ErrorAs(t, err, &sysErr)
+	require.ErrorAs(t, result.Err, &sysErr)
 	expectedSyscall := "connect"
 	if runtime.GOOS == "windows" {
 		expectedSyscall = "connectex"
 	}
 	require.Equal(t, expectedSyscall, sysErr.Syscall)
 
 	var errno syscall.Errno
-	require.ErrorAs(t, sysErr.Err, &errno)
+	require.ErrorAs(t, result.Err, &errno)
 	require.Equal(t, "ECONNREFUSED", errnoName(errno))
 }
 
@@ -104,24 +105,23 @@ func TestTestResolverStreamConnectivityReset(t *testing.T) {
 	}, &running)
 	defer listener.Close()
 
-	resolver := &transport.TCPEndpoint{Address: listener.Addr().String()}
-	_, err := TestResolverStreamConnectivity(context.Background(), resolver, "anything")
-
-	var testErr *TestError
-	require.ErrorAs(t, err, &testErr)
-	require.Equalf(t, "read", testErr.Op, "Wrong test operation. Error: %v", testErr.Err)
-	require.Equal(t, "ECONNRESET", testErr.PosixError)
+	resolver := NewTCPResolver(&transport.TCPStreamDialer{}, listener.Addr().String())
+	result, err := TestConnectivityWithResolver(context.Background(), resolver, "anything")
+	require.NoError(t, err)
+	require.NotNil(t, result)
+	require.Equalf(t, "receive", result.Op, "Wrong test operation. Error: %v", result.Err)
+	require.Equal(t, "ECONNRESET", result.PosixError)
 
 	var sysErr *os.SyscallError
-	require.ErrorAs(t, err, &sysErr)
+	require.ErrorAs(t, result.Err, &sysErr)
 	expectedSyscall := "read"
 	if runtime.GOOS == "windows" {
 		expectedSyscall = "wsarecv"
 	}
 	require.Equalf(t, expectedSyscall, sysErr.Syscall, "Wrong system call. Error: %v", sysErr)
 
 	var errno syscall.Errno
-	require.ErrorAs(t, err, &errno)
+	require.ErrorAs(t, result.Err, &errno)
 	require.Equal(t, "ECONNRESET", errnoName(errno))
 }
 
@@ -136,17 +136,16 @@ func TestTestStreamDialerEarlyClose(t *testing.T) {
 	}, &running)
 	defer listener.Close()
 
-	resolver := &transport.TCPEndpoint{Address: listener.Addr().String()}
-	_, err := TestResolverStreamConnectivity(context.Background(), resolver, "anything")
-
-	var testErr *TestError
-	require.ErrorAs(t, err, &testErr)
-	require.Equalf(t, "read", testErr.Op, "Wrong test operation. Error: %v", testErr.Err)
-	require.Equal(t, "", testErr.PosixError)
-	require.Error(t, err, "unexpected EOF")
+	resolver := NewTCPResolver(&transport.TCPStreamDialer{}, listener.Addr().String())
+	result, err := TestConnectivityWithResolver(context.Background(), resolver, "anything")
+	require.NoError(t, err)
+	require.NotNil(t, result)
+	require.Equalf(t, "receive", result.Op, "Wrong test operation. Error: %v", result.Err)
+	require.Equal(t, "", result.PosixError)
+	require.ErrorIs(t, result.Err, io.EOF)
 
 	var sysErr *os.SyscallError
-	require.False(t, errors.As(err, &sysErr))
+	require.False(t, errors.As(result.Err, &sysErr))
 }
 
 func TestTestResolverStreamConnectivityTimeout(t *testing.T) {
@@ -161,16 +160,16 @@ func TestTestResolverStreamConnectivityTimeout(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()
-	resolver := &transport.TCPEndpoint{Address: listener.Addr().String()}
-	_, err := TestResolverStreamConnectivity(ctx, resolver, "anything")
+	resolver := NewTCPResolver(&transport.TCPStreamDialer{}, listener.Addr().String())
+	result, err := TestConnectivityWithResolver(ctx, resolver, "anything")
+	require.NoError(t, err)
+	require.NotNil(t, result)
 
-	var testErr *TestError
-	require.ErrorAs(t, err, &testErr)
-	assert.Equalf(t, "read", testErr.Op, "Wrong test operation. Error: %v", testErr.Err)
+	assert.Equalf(t, "receive", result.Op, "Wrong test operation. Error: %v", result.Err)
 
-	assert.ErrorContains(t, err, "i/o timeout")
-	assert.True(t, isTimeout(err))
-	assert.Equalf(t, "ETIMEDOUT", testErr.PosixError, "Wrong posix error code. Error: %#v, %v", testErr.Err, testErr.Err.Error())
+	assert.ErrorContains(t, result.Err, "i/o timeout")
+	assert.True(t, isTimeout(result.Err))
+	assert.Equalf(t, "ETIMEDOUT", result.PosixError, "Wrong posix error code. Error: %#v, %v", result.Err, result.Err.Error())
 
 	timeout.Done()
 	listener.Close()
@@ -188,21 +187,22 @@ func TestTestPacketPacketConnectivityOk(t *testing.T) {
 		buf := make([]byte, 512)
 		n, clientAddr, err := server.ReadFrom(buf)
 		require.NoError(t, err)
-		var request dns.Msg
+		var request dnsmessage.Message
 		err = request.Unpack(buf[:n])
 		require.NoError(t, err)
 
-		var response dns.Msg
-		response.SetReply(&request)
-		responseBytes, err := response.Pack()
+		request.Response = true
+		request.RecursionAvailable = true
+		responseBytes, err := request.AppendPack(buf[0:0])
 		require.NoError(t, err)
 		_, err = server.WriteTo(responseBytes, clientAddr)
 		require.NoError(t, err)
 	}()
 
-	resolver := &transport.UDPEndpoint{Address: server.LocalAddr().String()}
-	_, err = TestResolverPacketConnectivity(context.Background(), resolver, "example.com")
+	resolver := NewUDPResolver(&transport.UDPPacketDialer{}, server.LocalAddr().String())
+	result, err := TestConnectivityWithResolver(context.Background(), resolver, "anything")
 	require.NoError(t, err)
+	require.Nil(t, result)
 }
 
 // TODO: Add more tests