Merge pull request #274 from LaurenceJJones/refactor/proxy-cleanup-basics

refactor(proxy): cleanup basics - constants, remove dead code, fix de…

clients/clients.go

@@ -161,8 +161,9 @@ func NewWireGuardService(interfaceName string, port uint16, mtu int, host string
 		useNativeInterface: useNativeInterface,
 	}
 
-	// Create the holepunch manager
-	service.holePunchManager = holepunch.NewManager(sharedBind, newtId, "newt", key.PublicKey().String(), nil)
+	// Create the holepunch manager with ResolveDomain function
+	// We'll need to pass a domain resolver function
+	service.holePunchManager = holepunch.NewManager(sharedBind, newtId, "newt", key.PublicKey().String())
 
 	// Register websocket handlers
 	wsClient.RegisterHandler("newt/wg/receive-config", service.handleConfig)

flake.nix

@@ -35,7 +35,7 @@
             inherit version;
             src = pkgs.nix-gitignore.gitignoreSource [ ] ./.;
 
-            vendorHash = "sha256-0eK4C42Upqpp01pfjW9+t3NKzadwVlGwwuWXhdpgDz4=";
+            vendorHash = "sha256-kmQM8Yy5TuOiNpMpUme/2gfE+vrhUK+0AphN+p71wGs=";
 
             nativeInstallCheckInputs = [ pkgs.versionCheckHook ];
 

go.mod

@@ -24,7 +24,7 @@ require (
 	golang.zx2c4.com/wireguard v0.0.0-20250521234502-f333402bd9cb
 	golang.zx2c4.com/wireguard/wgctrl v0.0.0-20241231184526-a9ab2273dd10
 	golang.zx2c4.com/wireguard/windows v0.5.3
-	google.golang.org/grpc v1.79.3
+	google.golang.org/grpc v1.79.1
 	gopkg.in/yaml.v3 v3.0.1
 	gvisor.dev/gvisor v0.0.0-20250503011706-39ed1f5ac29c
 	software.sslmate.com/src/go-pkcs12 v0.7.0

go.sum

@@ -159,8 +159,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20260209200024-4cfbd4190f57 h1:
 google.golang.org/genproto/googleapis/api v0.0.0-20260209200024-4cfbd4190f57/go.mod h1:kSJwQxqmFXeo79zOmbrALdflXQeAYcUbgS7PbpMknCY=
 google.golang.org/genproto/googleapis/rpc v0.0.0-20260209200024-4cfbd4190f57 h1:mWPCjDEyshlQYzBpMNHaEof6UX1PmHcaUODUywQ0uac=
 google.golang.org/genproto/googleapis/rpc v0.0.0-20260209200024-4cfbd4190f57/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ=
-google.golang.org/grpc v1.79.3 h1:sybAEdRIEtvcD68Gx7dmnwjZKlyfuc61Dyo9pGXXkKE=
-google.golang.org/grpc v1.79.3/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ=
+google.golang.org/grpc v1.79.1 h1:zGhSi45ODB9/p3VAawt9a+O/MULLl9dpizzNNpq7flY=
+google.golang.org/grpc v1.79.1/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ=
 google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE=
 google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

healthcheck/healthcheck.go

@@ -5,7 +5,9 @@ import (
 	"crypto/tls"
 	"encoding/json"
 	"fmt"
+	"net"
 	"net/http"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -365,11 +367,12 @@ func (m *Monitor) performHealthCheck(target *Target) {
 	target.LastCheck = time.Now()
 	target.LastError = ""
 
-	// Build URL
-	url := fmt.Sprintf("%s://%s", target.Config.Scheme, target.Config.Hostname)
+	// Build URL (use net.JoinHostPort to properly handle IPv6 addresses with ports)
+	host := target.Config.Hostname
 	if target.Config.Port > 0 {
-		url = fmt.Sprintf("%s:%d", url, target.Config.Port)
+		host = net.JoinHostPort(target.Config.Hostname, strconv.Itoa(target.Config.Port))
 	}
+	url := fmt.Sprintf("%s://%s", target.Config.Scheme, host)
 	if target.Config.Path != "" {
 		if !strings.HasPrefix(target.Config.Path, "/") {
 			url += "/"

holepunch/holepunch.go

@@ -27,17 +27,16 @@ type ExitNode struct {
 
 // Manager handles UDP hole punching operations
 type Manager struct {
-	mu          sync.Mutex
-	running     bool
-	stopChan    chan struct{}
-	sharedBind  *bind.SharedBind
-	ID          string
-	token       string
-	publicKey   string
-	clientType  string
-	exitNodes   map[string]ExitNode // key is endpoint
-	updateChan  chan struct{}        // signals the goroutine to refresh exit nodes
-	publicDNS []string
+	mu         sync.Mutex
+	running    bool
+	stopChan   chan struct{}
+	sharedBind *bind.SharedBind
+	ID         string
+	token      string
+	publicKey  string
+	clientType string
+	exitNodes  map[string]ExitNode // key is endpoint
+	updateChan chan struct{}       // signals the goroutine to refresh exit nodes
 
 	sendHolepunchInterval    time.Duration
 	sendHolepunchIntervalMin time.Duration
@@ -50,13 +49,12 @@ const defaultSendHolepunchIntervalMax = 60 * time.Second
 const defaultSendHolepunchIntervalMin = 1 * time.Second
 
 // NewManager creates a new hole punch manager
-func NewManager(sharedBind *bind.SharedBind, ID string, clientType string, publicKey string, publicDNS []string) *Manager {
+func NewManager(sharedBind *bind.SharedBind, ID string, clientType string, publicKey string) *Manager {
 	return &Manager{
 		sharedBind:               sharedBind,
 		ID:                       ID,
 		clientType:               clientType,
 		publicKey:                publicKey,
-		publicDNS:              publicDNS,
 		exitNodes:                make(map[string]ExitNode),
 		sendHolepunchInterval:    defaultSendHolepunchIntervalMin,
 		sendHolepunchIntervalMin: defaultSendHolepunchIntervalMin,
@@ -283,13 +281,7 @@ func (m *Manager) TriggerHolePunch() error {
 	// Send hole punch to all exit nodes
 	successCount := 0
 	for _, exitNode := range currentExitNodes {
-		var host string
-		var err error
-		if len(m.publicDNS) > 0 {
-			host, err = util.ResolveDomainUpstream(exitNode.Endpoint, m.publicDNS)
-		} else {
-			host, err = util.ResolveDomain(exitNode.Endpoint)
-		}
+		host, err := util.ResolveDomain(exitNode.Endpoint)
 		if err != nil {
 			logger.Warn("Failed to resolve endpoint %s: %v", exitNode.Endpoint, err)
 			continue
@@ -400,13 +392,7 @@ func (m *Manager) runMultipleExitNodes() {
 
 		var resolvedNodes []resolvedExitNode
 		for _, exitNode := range currentExitNodes {
-			var host string
-			var err error
-			if len(m.publicDNS) > 0 {
-				host, err = util.ResolveDomainUpstream(exitNode.Endpoint, m.publicDNS)
-			} else {
-				host, err = util.ResolveDomain(exitNode.Endpoint)
-			}
+			host, err := util.ResolveDomain(exitNode.Endpoint)
 			if err != nil {
 				logger.Warn("Failed to resolve endpoint %s: %v", exitNode.Endpoint, err)
 				continue

holepunch/tester.go

@@ -49,11 +49,10 @@ type cachedAddr struct {
 
 // HolepunchTester monitors holepunch connectivity using magic packets
 type HolepunchTester struct {
-	sharedBind  *bind.SharedBind
-	publicDNS []string
-	mu          sync.RWMutex
-	running     bool
-	stopChan    chan struct{}
+	sharedBind *bind.SharedBind
+	mu         sync.RWMutex
+	running    bool
+	stopChan   chan struct{}
 
 	// Pending requests waiting for responses (key: echo data as string)
 	pendingRequests sync.Map // map[string]*pendingRequest
@@ -85,10 +84,9 @@ type pendingRequest struct {
 }
 
 // NewHolepunchTester creates a new holepunch tester using the given SharedBind
-func NewHolepunchTester(sharedBind *bind.SharedBind, publicDNS []string) *HolepunchTester {
+func NewHolepunchTester(sharedBind *bind.SharedBind) *HolepunchTester {
 	return &HolepunchTester{
 		sharedBind:   sharedBind,
-		publicDNS:  publicDNS,
 		addrCache:    make(map[string]*cachedAddr),
 		addrCacheTTL: 5 * time.Minute, // Cache addresses for 5 minutes
 	}
@@ -171,13 +169,7 @@ func (t *HolepunchTester) resolveEndpoint(endpoint string) (*net.UDPAddr, error)
 	}
 
 	// Resolve the endpoint
-	var host string
-	var err error
-	if len(t.publicDNS) > 0 {
-		host, err = util.ResolveDomainUpstream(endpoint, t.publicDNS)
-	} else {
-		host, err = util.ResolveDomain(endpoint)
-	}
+	host, err := util.ResolveDomain(endpoint)
 	if err != nil {
 		host = endpoint
 	}

main.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/http/pprof"
 	"net/netip"
 	"os"
 	"os/signal"
@@ -147,6 +148,7 @@ var (
 	adminAddr         string
 	region            string
 	metricsAsyncBytes bool
+	pprofEnabled      bool
 	blueprintFile     string
 	noCloud           bool
 
@@ -225,6 +227,7 @@ func runNewtMain(ctx context.Context) {
 	adminAddrEnv := os.Getenv("NEWT_ADMIN_ADDR")
 	regionEnv := os.Getenv("NEWT_REGION")
 	asyncBytesEnv := os.Getenv("NEWT_METRICS_ASYNC_BYTES")
+	pprofEnabledEnv := os.Getenv("NEWT_PPROF_ENABLED")
 
 	disableClientsEnv := os.Getenv("DISABLE_CLIENTS")
 	disableClients = disableClientsEnv == "true"
@@ -390,6 +393,14 @@ func runNewtMain(ctx context.Context) {
 			metricsAsyncBytes = v
 		}
 	}
+	// pprof debug endpoint toggle
+	if pprofEnabledEnv == "" {
+		flag.BoolVar(&pprofEnabled, "pprof", false, "Enable pprof debug endpoints on admin server")
+	} else {
+		if v, err := strconv.ParseBool(pprofEnabledEnv); err == nil {
+			pprofEnabled = v
+		}
+	}
 	// Optional region flag (resource attribute)
 	if regionEnv == "" {
 		flag.StringVar(&region, "region", "", "Optional region resource attribute (also NEWT_REGION)")
@@ -485,6 +496,14 @@ func runNewtMain(ctx context.Context) {
 		if tel.PrometheusHandler != nil {
 			mux.Handle("/metrics", tel.PrometheusHandler)
 		}
+		if pprofEnabled {
+			mux.HandleFunc("/debug/pprof/", pprof.Index)
+			mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
+			mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
+			mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
+			mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
+			logger.Info("pprof debugging enabled on %s/debug/pprof/", tcfg.AdminAddr)
+		}
 		admin := &http.Server{
 			Addr:              tcfg.AdminAddr,
 			Handler:           otelhttp.NewHandler(mux, "newt-admin"),

proxy/manager.go

@@ -21,7 +21,10 @@ import (
 	"gvisor.dev/gvisor/pkg/tcpip/adapters/gonet"
 )
 
-const errUnsupportedProtoFmt = "unsupported protocol: %s"
+const (
+	errUnsupportedProtoFmt = "unsupported protocol: %s"
+	maxUDPPacketSize       = 65507
+)
 
 // Target represents a proxy target with its address and port
 type Target struct {
@@ -105,13 +108,9 @@ func classifyProxyError(err error) string {
 	if errors.Is(err, net.ErrClosed) {
 		return "closed"
 	}
-	if ne, ok := err.(net.Error); ok {
-		if ne.Timeout() {
-			return "timeout"
-		}
-		if ne.Temporary() {
-			return "temporary"
-		}
+	var ne net.Error
+	if errors.As(err, &ne) && ne.Timeout() {
+		return "timeout"
 	}
 	msg := strings.ToLower(err.Error())
 	switch {
@@ -437,14 +436,6 @@ func (pm *ProxyManager) Stop() error {
 		pm.udpConns = append(pm.udpConns[:i], pm.udpConns[i+1:]...)
 	}
 
-	// // Clear the target maps
-	// for k := range pm.tcpTargets {
-	// 	delete(pm.tcpTargets, k)
-	// }
-	// for k := range pm.udpTargets {
-	// 	delete(pm.udpTargets, k)
-	// }
-
 	// Give active connections a chance to close gracefully
 	time.Sleep(100 * time.Millisecond)
 
@@ -498,7 +489,7 @@ func (pm *ProxyManager) handleTCPProxy(listener net.Listener, targetAddr string)
 			if !pm.running {
 				return
 			}
-			if ne, ok := err.(net.Error); ok && !ne.Temporary() {
+			if errors.Is(err, net.ErrClosed) {
 				logger.Info("TCP listener closed, stopping proxy handler for %v", listener.Addr())
 				return
 			}
@@ -564,7 +555,7 @@ func (pm *ProxyManager) handleTCPProxy(listener net.Listener, targetAddr string)
 }
 
 func (pm *ProxyManager) handleUDPProxy(conn *gonet.UDPConn, targetAddr string) {
-	buffer := make([]byte, 65507) // Max UDP packet size
+	buffer := make([]byte, maxUDPPacketSize) // Max UDP packet size
 	clientConns := make(map[string]*net.UDPConn)
 	var clientsMutex sync.RWMutex
 
@@ -583,7 +574,7 @@ func (pm *ProxyManager) handleUDPProxy(conn *gonet.UDPConn, targetAddr string) {
 			}
 
 			// Check for connection closed conditions
-			if err == io.EOF || strings.Contains(err.Error(), "use of closed network connection") {
+			if errors.Is(err, io.EOF) || errors.Is(err, net.ErrClosed) {
 				logger.Info("UDP connection closed, stopping proxy handler")
 
 				// Clean up existing client connections
@@ -662,10 +653,14 @@ func (pm *ProxyManager) handleUDPProxy(conn *gonet.UDPConn, targetAddr string) {
 					telemetry.IncProxyConnectionEvent(context.Background(), tunnelID, "udp", telemetry.ProxyConnectionClosed)
 				}()
 
-				buffer := make([]byte, 65507)
+				buffer := make([]byte, maxUDPPacketSize)
 				for {
 					n, _, err := targetConn.ReadFromUDP(buffer)
 					if err != nil {
+						// Connection closed is normal during cleanup
+						if errors.Is(err, net.ErrClosed) || errors.Is(err, io.EOF) {
+							return // defer will handle cleanup, result stays "success"
+						}
 						logger.Error("Error reading from target: %v", err)
 						result = "failure"
 						return // defer will handle cleanup

util/util.go

@@ -1,7 +1,6 @@
 package util
 
 import (
-	"context"
 	"encoding/base64"
 	"encoding/binary"
 	"encoding/hex"
@@ -15,99 +14,6 @@ import (
 	"golang.zx2c4.com/wireguard/device"
 )
 
-func ResolveDomainUpstream(domain string, publicDNS []string) (string, error) {
-	// trim whitespace
-	domain = strings.TrimSpace(domain)
-
-	// Remove any protocol prefix if present (do this first, before splitting host/port)
-	domain = strings.TrimPrefix(domain, "http://")
-	domain = strings.TrimPrefix(domain, "https://")
-
-	// if there are any trailing slashes, remove them
-	domain = strings.TrimSuffix(domain, "/")
-
-	// Check if there's a port in the domain
-	host, port, err := net.SplitHostPort(domain)
-	if err != nil {
-		// No port found, use the domain as is
-		host = domain
-		port = ""
-	}
-
-	// Check if host is already an IP address (IPv4 or IPv6)
-	// For IPv6, the host from SplitHostPort will already have brackets stripped
-	// but if there was no port, we need to handle bracketed IPv6 addresses
-	cleanHost := strings.TrimPrefix(strings.TrimSuffix(host, "]"), "[")
-	if ip := net.ParseIP(cleanHost); ip != nil {
-		// It's already an IP address, no need to resolve
-		ipAddr := ip.String()
-		if port != "" {
-			return net.JoinHostPort(ipAddr, port), nil
-		}
-		return ipAddr, nil
-	}
-
-	// Lookup IP addresses using the upstream DNS servers if provided
-	var ips []net.IP
-	if len(publicDNS) > 0 {
-		var lastErr error
-		for _, server := range publicDNS {
-			// Ensure the upstream DNS address has a port
-			dnsAddr := server
-			if _, _, err := net.SplitHostPort(dnsAddr); err != nil {
-				// No port specified, default to 53
-				dnsAddr = net.JoinHostPort(server, "53")
-			}
-
-			resolver := &net.Resolver{
-				PreferGo: true,
-				Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
-					d := net.Dialer{}
-					return d.DialContext(ctx, "udp", dnsAddr)
-				},
-			}
-			ips, lastErr = resolver.LookupIP(context.Background(), "ip", host)
-			if lastErr == nil {
-				break
-			}
-		}
-		if lastErr != nil {
-			return "", fmt.Errorf("DNS lookup failed using all upstream servers: %v", lastErr)
-		}
-	} else {
-		ips, err = net.LookupIP(host)
-		if err != nil {
-			return "", fmt.Errorf("DNS lookup failed: %v", err)
-		}
-	}
-
-	if len(ips) == 0 {
-		return "", fmt.Errorf("no IP addresses found for domain %s", host)
-	}
-
-	// Get the first IPv4 address if available
-	var ipAddr string
-	for _, ip := range ips {
-		if ipv4 := ip.To4(); ipv4 != nil {
-			ipAddr = ipv4.String()
-			break
-		}
-	}
-
-	// If no IPv4 found, use the first IP (might be IPv6)
-	if ipAddr == "" {
-		ipAddr = ips[0].String()
-	}
-
-	// Add port back if it existed
-	if port != "" {
-		ipAddr = net.JoinHostPort(ipAddr, port)
-	}
-
-	return ipAddr, nil
-}
-
-
 func ResolveDomain(domain string) (string, error) {
 	// trim whitespace
 	domain = strings.TrimSpace(domain)