Opt-in retries for HTTP origin connections

cmd/cloudflared/tunnel/cmd.go

@@ -975,6 +975,12 @@ func configureProxyFlags(shouldHide bool) []cli.Flag {
 			Value:  time.Second * 30,
 			Hidden: shouldHide,
 		}),
+		altsrc.NewDurationFlag(&cli.DurationFlag{
+			Name:    ingress.ProxyConnectRetryTimeoutFlag,
+			Usage:   "Total time to retry refused HTTP origin connections or missing Unix sockets (0 disables retries). Applies to --url or --unix-socket; for ingress rules, set originRequest.connectRetryTimeout.",
+			EnvVars: []string{"TUNNEL_PROXY_CONNECT_RETRY_TIMEOUT"},
+			Hidden:  shouldHide,
+		}),
 		altsrc.NewDurationFlag(&cli.DurationFlag{
 			Name:   ingress.ProxyTLSTimeoutFlag,
 			Usage:  legacyTunnelFlag("HTTP proxy timeout for completing a TLS handshake"),

cmd/cloudflared/tunnel/origin_retry_test.go

@@ -0,0 +1,62 @@
+package tunnel
+
+import (
+	"testing"
+	"time"
+
+	"github.com/rs/zerolog"
+	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
+
+	"github.com/cloudflare/cloudflared/config"
+	"github.com/cloudflare/cloudflared/ingress"
+)
+
+func TestOriginConnectRetryFlag(t *testing.T) {
+	testCases := []struct {
+		name     string
+		env      string
+		flag     string
+		expected time.Duration
+		wantErr  bool
+	}{
+		{name: "disabled by default"},
+		{name: "environment", env: "500ms", expected: 500 * time.Millisecond},
+		{name: "flag", flag: "500ms", expected: 500 * time.Millisecond},
+		{name: "flag overrides environment", env: "500ms", flag: "2s", expected: 2 * time.Second},
+		{name: "flag disables environment", env: "500ms", flag: "0s"},
+		{name: "negative flag", flag: "-1s", wantErr: true},
+		{name: "negative environment", env: "-1s", wantErr: true},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv("TUNNEL_PROXY_CONNECT_RETRY_TIMEOUT", tc.env)
+			for _, origin := range [][]string{{"--url", "http://localhost:8000"}, {"--unix-socket", "/tmp/app.sock"}} {
+				t.Run(origin[0], func(t *testing.T) {
+					app := cli.NewApp()
+					app.Flags = configureProxyFlags(false)
+					app.Action = func(c *cli.Context) error {
+						log := zerolog.Nop()
+						rules, err := ingress.ParseIngressFromConfigAndCLI(&config.Configuration{}, c, &log)
+						if err != nil {
+							return err
+						}
+						require.Len(t, rules.Rules, 1)
+						require.Equal(t, tc.expected, rules.Rules[0].Config.ConnectRetryTimeout.Duration)
+						return nil
+					}
+					args := append([]string{"cloudflared"}, origin...)
+					if tc.flag != "" {
+						args = append(args, "--proxy-connect-retry-timeout", tc.flag)
+					}
+					err := app.Run(args)
+					if tc.wantErr {
+						require.ErrorContains(t, err, "connectRetryTimeout must not be negative")
+						return
+					}
+					require.NoError(t, err)
+				})
+			}
+		})
+	}
+}

config/configuration.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"math"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -35,7 +36,7 @@ var (
 	defaultUserConfigDirs = []string{"~/.cloudflared", "~/.cloudflare-warp", "~/cloudflare-warp"}
 	defaultNixConfigDirs  = []string{"/etc/cloudflared", DefaultUnixConfigLocation}
 
-	ErrNoConfigFile = fmt.Errorf("Cannot determine default configuration path. No file %v in %v", DefaultConfigFiles, DefaultConfigSearchDirectories())
+	ErrNoConfigFile = fmt.Errorf("Cannot determine default configuration path. No file %v in %v", DefaultConfigFiles, DefaultConfigSearchDirectories()) //nolint:staticcheck // Preserving the existing user-facing error text.
 )
 
 const (
@@ -49,7 +50,7 @@ func DefaultConfigDirectory() string {
 		path := os.Getenv("CFDPATH")
 		if path == "" {
 			path = filepath.Join(os.Getenv("ProgramFiles(x86)"), "cloudflared")
-			if _, err := os.Stat(path); os.IsNotExist(err) { // doesn't exist, so return an empty failure string
+			if _, err := os.Stat(path); os.IsNotExist(err) { //nolint:gosec // Installation path comes from the local process environment.
 				return ""
 			}
 		}
@@ -87,7 +88,7 @@ func DefaultConfigSearchDirectories() []string {
 
 // FileExists checks to see if a file exist at the provided path.
 func FileExists(path string) (bool, error) {
-	f, err := os.Open(path)
+	f, err := os.Open(path) //nolint:gosec // Checking a caller-supplied configuration path.
 	if err != nil {
 		if os.IsNotExist(err) {
 			// ignore missing files
@@ -126,19 +127,19 @@ func FindOrCreateConfigPath() string {
 	if path == "" {
 		// create the default directory if it doesn't exist
 		path = DefaultConfigPath()
-		if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil {
+		if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil { //nolint:gosec // Preserving existing umask-controlled directory permissions.
 			return ""
 		}
 
 		// write a new config file out
-		file, err := os.Create(path)
+		file, err := os.Create(path) //nolint:gosec // The path is selected by FindDefaultConfigPath or DefaultConfigPath.
 		if err != nil {
 			return ""
 		}
-		defer file.Close()
+		defer func() { _ = file.Close() }()
 
 		logDir := DefaultLogDirectory()
-		_ = os.MkdirAll(logDir, os.ModePerm) // try and create it. Doesn't matter if it succeed or not, only byproduct will be no logs
+		_ = os.MkdirAll(logDir, os.ModePerm) //nolint:gosec // Best-effort log directory creation with existing umask-controlled permissions.
 
 		c := Root{
 			LogDirectory: logDir,
@@ -187,10 +188,12 @@ type UnvalidatedIngressRule struct {
 // config.
 // Note:
 // - To specify a time.Duration in go-yaml, use e.g. "3s" or "24h".
-// - To specify a time.Duration in json, use int64 of the nanoseconds
+// - To specify a time.Duration in JSON, use seconds (e.g. 0.5 or 3).
 type OriginRequestConfig struct {
 	// HTTP proxy timeout for establishing a new connection
 	ConnectTimeout *CustomDuration `yaml:"connectTimeout" json:"connectTimeout,omitempty"`
+	// Total time to retry refused HTTP origin connections or missing Unix sockets. Zero disables retries.
+	ConnectRetryTimeout *CustomDuration `yaml:"connectRetryTimeout" json:"connectRetryTimeout,omitempty"`
 	// HTTP proxy timeout for completing a TLS handshake
 	TLSTimeout *CustomDuration `yaml:"tlsTimeout" json:"tlsTimeout,omitempty"`
 	// HTTP proxy TCP keepalive duration
@@ -391,15 +394,15 @@ func ReadConfigFile(c *cli.Context, log *zerolog.Logger) (settings *configFileSe
 	}
 
 	log.Debug().Msgf("Loading configuration from %s", configFile)
-	file, err := os.Open(configFile)
+	file, err := os.Open(configFile) //nolint:gosec // Config path is explicitly selected by the local operator.
 	if err != nil {
 		// If does not exist and config file was not specificly specified then return ErrNoConfigFile found.
 		if os.IsNotExist(err) && !c.IsSet("config") {
 			err = ErrNoConfigFile
 		}
 		return nil, "", err
 	}
-	defer file.Close()
+	defer func() { _ = file.Close() }()
 	if err := yaml.NewDecoder(file).Decode(&configuration); err != nil {
 		if err == io.EOF {
 			log.Error().Msgf("Configuration file %s was empty", configFile)
@@ -410,7 +413,8 @@ func ReadConfigFile(c *cli.Context, log *zerolog.Logger) (settings *configFileSe
 	configuration.sourceFile = configFile
 
 	// Parse it again, with strict mode, to find warnings.
-	if file, err := os.Open(configFile); err == nil {
+	if file, err := os.Open(configFile); err == nil { //nolint:gosec // Re-reading the same operator-selected configuration file.
+		defer func() { _ = file.Close() }()
 		decoder := yaml.NewDecoder(file)
 		decoder.KnownFields(true)
 		var unusedConfig configFileSettings
@@ -422,31 +426,31 @@ func ReadConfigFile(c *cli.Context, log *zerolog.Logger) (settings *configFileSe
 	return &configuration, warnings, nil
 }
 
-// A CustomDuration is a Duration that has custom serialization for JSON.
-// JSON in Javascript assumes that int fields are 32 bits and Duration fields are deserialized assuming that numbers
-// are in nanoseconds, which in 32bit integers limits to just 2 seconds.
-// This type assumes that when serializing/deserializing from JSON, that the number is in seconds, while it maintains
-// the YAML serde assumptions.
+// A duration encoded as seconds in JSON and as a Go duration string in YAML.
 type CustomDuration struct {
 	time.Duration
 }
 
 func (s CustomDuration) MarshalJSON() ([]byte, error) {
-	return json.Marshal(s.Duration.Seconds())
+	return json.Marshal(s.Seconds())
 }
 
 func (s *CustomDuration) UnmarshalJSON(data []byte) error {
-	seconds, err := strconv.ParseInt(string(data), 10, 64)
+	seconds, err := strconv.ParseFloat(string(data), 64)
 	if err != nil {
 		return err
 	}
 
-	s.Duration = time.Duration(seconds * int64(time.Second))
+	nanoseconds := math.Round(seconds * float64(time.Second))
+	if math.IsNaN(nanoseconds) || nanoseconds >= float64(math.MaxInt64) || nanoseconds < float64(math.MinInt64) {
+		return fmt.Errorf("duration %s seconds is out of range", data)
+	}
+	s.Duration = time.Duration(nanoseconds)
 	return nil
 }
 
 func (s *CustomDuration) MarshalYAML() (interface{}, error) {
-	return s.Duration.String(), nil
+	return s.String(), nil
 }
 
 func (s *CustomDuration) UnmarshalYAML(unmarshal func(interface{}) error) error {

config/duration_test.go

@@ -0,0 +1,51 @@
+package config
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCustomDurationJSON(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		json     string
+		duration time.Duration
+	}{
+		{"0", 0},
+		{"0.000000001", time.Nanosecond},
+		{"0.5", 500 * time.Millisecond},
+		{"1.001", 1001 * time.Millisecond},
+		{"-0.5", -500 * time.Millisecond},
+		{"1", time.Second},
+		{"30", 30 * time.Second},
+		{"3600", time.Hour},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.json, func(t *testing.T) {
+			t.Parallel()
+			var decoded CustomDuration
+			err := json.Unmarshal([]byte(tc.json), &decoded)
+			require.NoError(t, err)
+			require.Equal(t, tc.duration, decoded.Duration)
+
+			data, err := json.Marshal(CustomDuration{Duration: tc.duration})
+			require.NoError(t, err)
+			require.JSONEq(t, tc.json, string(data))
+		})
+	}
+}
+
+func TestCustomDurationRejectsInvalidJSON(t *testing.T) {
+	t.Parallel()
+	for _, data := range []string{`null`, `"1s"`, `1e100`, `-1e100`, `9223372037`, `-9223372037`} {
+		t.Run(data, func(t *testing.T) {
+			t.Parallel()
+			var decoded CustomDuration
+			err := json.Unmarshal([]byte(data), &decoded)
+			require.Error(t, err)
+		})
+	}
+}

ingress/config.go

@@ -26,6 +26,7 @@ const (
 	SSHServerFlag                 = "ssh-server"
 	Socks5Flag                    = "socks5"
 	ProxyConnectTimeoutFlag       = "proxy-connect-timeout"
+	ProxyConnectRetryTimeoutFlag  = "proxy-connect-retry-timeout"
 	ProxyTLSTimeoutFlag           = "proxy-tls-timeout"
 	ProxyTCPKeepAliveFlag         = "proxy-tcp-keepalive"
 	ProxyNoHappyEyeballsFlag      = "proxy-no-happy-eyeballs"
@@ -121,6 +122,7 @@ func (rc *RemoteConfig) UnmarshalJSON(b []byte) error {
 
 func originRequestFromSingleRule(c *cli.Context) OriginRequestConfig {
 	var connectTimeout = defaultHTTPConnectTimeout
+	var connectRetryTimeout config.CustomDuration
 	var tlsTimeout = defaultTLSTimeout
 	var tcpKeepAlive = defaultTCPKeepAlive
 	var noHappyEyeballs bool
@@ -140,6 +142,9 @@ func originRequestFromSingleRule(c *cli.Context) OriginRequestConfig {
 	if flag := ProxyConnectTimeoutFlag; c.IsSet(flag) {
 		connectTimeout = config.CustomDuration{Duration: c.Duration(flag)}
 	}
+	if flag := ProxyConnectRetryTimeoutFlag; c.IsSet(flag) {
+		connectRetryTimeout = config.CustomDuration{Duration: c.Duration(flag)}
+	}
 	if flag := ProxyTLSTimeoutFlag; c.IsSet(flag) {
 		tlsTimeout = config.CustomDuration{Duration: c.Duration(flag)}
 	}
@@ -193,6 +198,7 @@ func originRequestFromSingleRule(c *cli.Context) OriginRequestConfig {
 
 	return OriginRequestConfig{
 		ConnectTimeout:         connectTimeout,
+		ConnectRetryTimeout:    connectRetryTimeout,
 		TLSTimeout:             tlsTimeout,
 		TCPKeepAlive:           tcpKeepAlive,
 		NoHappyEyeballs:        noHappyEyeballs,
@@ -224,6 +230,9 @@ func originRequestFromConfig(c config.OriginRequestConfig) OriginRequestConfig {
 	if c.ConnectTimeout != nil {
 		out.ConnectTimeout = *c.ConnectTimeout
 	}
+	if c.ConnectRetryTimeout != nil {
+		out.ConnectRetryTimeout = *c.ConnectRetryTimeout
+	}
 	if c.TLSTimeout != nil {
 		out.TLSTimeout = *c.TLSTimeout
 	}
@@ -292,6 +301,8 @@ func originRequestFromConfig(c config.OriginRequestConfig) OriginRequestConfig {
 type OriginRequestConfig struct {
 	// HTTP proxy timeout for establishing a new connection
 	ConnectTimeout config.CustomDuration `yaml:"connectTimeout" json:"connectTimeout"`
+	// Total time to retry refused HTTP origin connections or missing Unix sockets. Zero disables retries.
+	ConnectRetryTimeout config.CustomDuration `yaml:"connectRetryTimeout" json:"connectRetryTimeout,omitzero"`
 	// HTTP proxy timeout for completing a TLS handshake
 	TLSTimeout config.CustomDuration `yaml:"tlsTimeout" json:"tlsTimeout"`
 	// HTTP proxy TCP keepalive duration
@@ -341,6 +352,12 @@ func (defaults *OriginRequestConfig) setConnectTimeout(overrides config.OriginRe
 	}
 }
 
+func (defaults *OriginRequestConfig) setConnectRetryTimeout(overrides config.OriginRequestConfig) {
+	if val := overrides.ConnectRetryTimeout; val != nil {
+		defaults.ConnectRetryTimeout = *val
+	}
+}
+
 func (defaults *OriginRequestConfig) setTLSTimeout(overrides config.OriginRequestConfig) {
 	if val := overrides.TLSTimeout; val != nil {
 		defaults.TLSTimeout = *val
@@ -467,6 +484,7 @@ func (defaults *OriginRequestConfig) setAccess(overrides config.OriginRequestCon
 func setConfig(defaults OriginRequestConfig, overrides config.OriginRequestConfig) OriginRequestConfig {
 	cfg := defaults
 	cfg.setConnectTimeout(overrides)
+	cfg.setConnectRetryTimeout(overrides)
 	cfg.setTLSTimeout(overrides)
 	cfg.setNoHappyEyeballs(overrides)
 	cfg.setKeepAliveConnections(overrides)
@@ -491,6 +509,7 @@ func setConfig(defaults OriginRequestConfig, overrides config.OriginRequestConfi
 
 func ConvertToRawOriginConfig(c OriginRequestConfig) config.OriginRequestConfig {
 	var connectTimeout *config.CustomDuration
+	var connectRetryTimeout *config.CustomDuration
 	var tlsTimeout *config.CustomDuration
 	var tcpKeepAlive *config.CustomDuration
 	var keepAliveConnections *int
@@ -501,6 +520,9 @@ func ConvertToRawOriginConfig(c OriginRequestConfig) config.OriginRequestConfig
 	if c.ConnectTimeout != defaultHTTPConnectTimeout {
 		connectTimeout = &c.ConnectTimeout
 	}
+	if c.ConnectRetryTimeout.Duration != 0 {
+		connectRetryTimeout = &c.ConnectRetryTimeout
+	}
 	if c.TLSTimeout != defaultTLSTimeout {
 		tlsTimeout = &c.TLSTimeout
 	}
@@ -522,6 +544,7 @@ func ConvertToRawOriginConfig(c OriginRequestConfig) config.OriginRequestConfig
 
 	return config.OriginRequestConfig{
 		ConnectTimeout:         connectTimeout,
+		ConnectRetryTimeout:    connectRetryTimeout,
 		TLSTimeout:             tlsTimeout,
 		TCPKeepAlive:           tcpKeepAlive,
 		NoHappyEyeballs:        defaultBoolToNil(c.NoHappyEyeballs),
@@ -544,7 +567,7 @@ func ConvertToRawOriginConfig(c OriginRequestConfig) config.OriginRequestConfig
 }
 
 func convertToRawIPRules(ipRules []ipaccess.Rule) []config.IngressIPRule {
-	result := make([]config.IngressIPRule, 0)
+	result := make([]config.IngressIPRule, 0, len(ipRules))
 	for _, r := range ipRules {
 		cidr := r.StringCIDR()