Code review with proposed changes

cmd/cloudflared/config/model.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"io"
 	"strings"
+
+	"github.com/cloudflare/cloudflared/tunneldns"
 )
 
 // Forwarder represents a client side listener to forward traffic to the edge
@@ -25,12 +27,12 @@ type Tunnel struct {
 
 // DNSResolver represents a client side DNS resolver
 type DNSResolver struct {
-	Enabled    bool     `json:"enabled"`
+	Enabled                bool     `json:"enabled"`
-	Address    string   `json:"address,omitempty"`
+	Address                string   `json:"address,omitempty"`
-	Port       uint16   `json:"port,omitempty"`
+	Port                   uint16   `json:"port,omitempty"`
-	Upstreams  []string `json:"upstreams,omitempty"`
+	Upstreams              []string `json:"upstreams,omitempty"`
-	Bootstraps []string `json:"bootstraps,omitempty"`
+	Bootstraps             []string `json:"bootstraps,omitempty"`
-	MaxUpstreamConnections int `json:"max_upstream_connections,omitempty"`
+	MaxUpstreamConnections int      `json:"max_upstream_connections,omitempty"`
 }
 
 // Root is the base options to configure the service
@@ -102,10 +104,10 @@ func (r *DNSResolver) BootstrapsOrDefault() []string {
 	return []string{"https://162.159.36.1/dns-query", "https://162.159.46.1/dns-query", "https://[2606:4700:4700::1111]/dns-query", "https://[2606:4700:4700::1001]/dns-query"}
 }
 
-// MaxUpstreamConnectionsOrDefault return the max upstream connections or returns the default if 0
+// MaxUpstreamConnectionsOrDefault return the max upstream connections or returns the default if negative
 func (r *DNSResolver) MaxUpstreamConnectionsOrDefault() int {
-    if r.MaxUpstreamConnections >= 0 {
+	if r.MaxUpstreamConnections >= 0 {
-        return r.MaxUpstreamConnections
+		return r.MaxUpstreamConnections
-    }
+	}
-    return 0
+	return tunneldns.MaxUpstreamConnsDefault
 }

cmd/cloudflared/tunnel/server.go

@@ -1,6 +1,8 @@
 package tunnel
 
 import (
+	"fmt"
+
 	"github.com/cloudflare/cloudflared/tunneldns"
 
 	"github.com/pkg/errors"
@@ -13,9 +15,9 @@ func runDNSProxyServer(c *cli.Context, dnsReadySignal, shutdownC chan struct{},
 	if port <= 0 || port > 65535 {
 		return errors.New("The 'proxy-dns-port' must be a valid port number in <1, 65535> range.")
 	}
-	maxUpstreamConnections := c.Int("proxy-dns-max-upstream-conns")
+	maxUpstreamConnections := c.Int(tunneldns.MaxUpstreamConnsFlag)
 	if maxUpstreamConnections < 0 {
-		return errors.New("'proxy-dns-max-upstream-conns' must be 0 or higher")
+		return fmt.Errorf("'%s' must be 0 or higher", tunneldns.MaxUpstreamConnsFlag)
 	}
 	listener, err := tunneldns.CreateListener(c.String("proxy-dns-address"), uint16(port), c.StringSlice("proxy-dns-upstream"), c.StringSlice("proxy-dns-bootstrap"), maxUpstreamConnections, log)
 	if err != nil {

tunneldns/https_upstream.go

@@ -23,11 +23,10 @@ const (
 
 // UpstreamHTTPS is the upstream implementation for DNS over HTTPS service
 type UpstreamHTTPS struct {
-	client         *http.Client
+	client     *http.Client
-	endpoint       *url.URL
+	endpoint   *url.URL
-	bootstraps     []string
+	bootstraps []string
-	maxConnections int
+	log        *zerolog.Logger
-	log            *zerolog.Logger
 }
 
 // NewUpstreamHTTPS creates a new DNS over HTTPS upstream from endpoint
@@ -123,7 +122,7 @@ func configureBootstrap(bootstrap string) (*url.URL, *http.Client, error) {
 		return nil, nil, fmt.Errorf("bootstrap address of %s must be an IP address", b.Hostname())
 	}
 
-	return b, configureClient(b.Hostname(), 0), nil
+	return b, configureClient(b.Hostname(), MaxUpstreamConnsDefault), nil
 }
 
 // configureClient will configure a HTTPS client for upstream DoH requests

tunneldns/tunnel.go

@@ -21,8 +21,10 @@ import (
 )
 
 const (
-	LogFieldAddress = "address"
+	LogFieldAddress         = "address"
-	LogFieldURL     = "url"
+	LogFieldURL             = "url"
+	MaxUpstreamConnsFlag    = "max-upstream-conns"
+	MaxUpstreamConnsDefault = 10
 )
 
 // Listener is an adapter between CoreDNS server and Warp runnable
@@ -69,9 +71,9 @@ func Command(hidden bool) *cli.Command {
 				EnvVars: []string{"TUNNEL_DNS_BOOTSTRAP"},
 			},
 			&cli.IntFlag{
-				Name:    "max-upstream-conns",
+				Name:    MaxUpstreamConnsFlag,
-				Usage:   "Maximum concurrent connections to upstream, unlimited by default",
+				Usage:   "Maximum concurrent connections to upstream. Setting to 0 means unlimited.",
-				Value:   0,
+				Value:   MaxUpstreamConnsDefault,
 				EnvVars: []string{"TUNNEL_DNS_MAX_UPSTREAM_CONNS"},
 			},
 		},
@@ -96,7 +98,7 @@ func Run(c *cli.Context) error {
 		uint16(c.Uint("port")),
 		c.StringSlice("upstream"),
 		c.StringSlice("bootstrap"),
-		c.Int("max-upstream-conns"),
+		c.Int(MaxUpstreamConnsFlag),
 		log,
 	)