Allow to configure server's read/write/idle timeouts (#36)

* allow to configure server's read/write/idle timeouts

* rm unused func

* specify where timeout should be applied

* add tests to check TimeoutCfg calculation

Author: hagen1778

README.md

@@ -430,6 +430,19 @@ server:
     # By default requests are accepted from all the IPs.
     allowed_networks: ["office", "reporting-apps", "1.2.3.4"]
 
+    # ReadTimeout is the maximum duration for proxy to reading the entire
+    # request, including the body.
+    # Default value is 1m
+    read_timeout: 5m
+
+    # WriteTimeout is the maximum duration for proxy before timing out writes of the response.
+    # Default is largest MaxExecutionTime + MaxQueueTime value from Users or Clusters
+    write_timeout: 10m
+
+    # IdleTimeout is the maximum amount of time for proxy to wait for the next request.
+    # Default is 10m
+    idle_timeout: 20m
+
   # Configs for input https interface.
   # The interface works only if this section is present.
   https:

config/README.md

@@ -106,6 +106,17 @@ listen_addr: <addr>
 # List of networks or network_groups access is allowed from
 # Each list item could be IP address or subnet mask
 allowed_networks: <network_groups>, <networks> ... | optional
+
+# ReadTimeout is the maximum duration for reading the entire
+# request, including the body.
+read_timeout: <duration> | optional | default = 1m
+
+# WriteTimeout is the maximum duration before timing out writes of the response.
+# Default is largest MaxExecutionTime + MaxQueueTime value from Users or Clusters
+write_timeout: <duration> | optional
+
+// IdleTimeout is the maximum amount of time to wait for the next request.
+idle_timeout: <duration> | optional | default = 10m
 ```
 
 ### <https_config>
@@ -117,6 +128,17 @@ listen_addr: <addr> | optional | default = `:443`
 # Each list item could be IP address or subnet mask
 allowed_networks: <network_groups>, <networks> ... | optional
 
+# ReadTimeout is the maximum duration for proxy to reading the entire
+# request, including the body.
+read_timeout: <duration> | optional | default = 1m
+
+# WriteTimeout is the maximum duration for proxy before timing out writes of the response.
+# Default is largest MaxExecutionTime + MaxQueueTime value from Users or Clusters
+write_timeout: <duration> | optional
+
+// IdleTimeout is the maximum amount of time for proxy to wait for the next request.
+idle_timeout: <duration> | optional | default = 10m
+
 # Certificate and key files for client cert authentication to the server
 cert_file: <string> | optional
 key_file: <string> | optional

config/config.go

@@ -113,6 +113,22 @@ func (s *Server) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return checkOverflow(s.XXX, "server")
 }
 
+// TimeoutCfg contains configurable http.Server timeouts
+type TimeoutCfg struct {
+	// ReadTimeout is the maximum duration for reading the entire
+	// request, including the body.
+	// Default value is 1m
+	ReadTimeout Duration `yaml:"read_timeout,omitempty"`
+
+	// WriteTimeout is the maximum duration before timing out writes of the response.
+	// Default is largest MaxExecutionTime + MaxQueueTime value from Users or Clusters
+	WriteTimeout Duration `yaml:"write_timeout,omitempty"`
+
+	// IdleTimeout is the maximum amount of time to wait for the next request.
+	// Default is 10m
+	IdleTimeout Duration `yaml:"idle_timeout,omitempty"`
+}
+
 // HTTP describes configuration for server to listen HTTP connections
 type HTTP struct {
 	// TCP address to listen to for http
@@ -128,6 +144,8 @@ type HTTP struct {
 	// Whether to support Autocert handler for http-01 challenge
 	ForceAutocertHandler bool
 
+	TimeoutCfg `yaml:",inline"`
+
 	// Catches all undefined fields and must be empty after parsing.
 	XXX map[string]interface{} `yaml:",inline"`
 }
@@ -138,6 +156,12 @@ func (c *HTTP) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if err := unmarshal((*plain)(c)); err != nil {
 		return err
 	}
+	if c.ReadTimeout == 0 {
+		c.ReadTimeout = Duration(time.Minute)
+	}
+	if c.IdleTimeout == 0 {
+		c.IdleTimeout = Duration(time.Minute * 10)
+	}
 	return checkOverflow(c.XXX, "http")
 }
 
@@ -162,6 +186,8 @@ type HTTPS struct {
 	// if omitted or zero - no limits would be applied
 	AllowedNetworks Networks `yaml:"-"`
 
+	TimeoutCfg `yaml:",inline"`
+
 	// Catches all undefined fields and must be empty after parsing.
 	XXX map[string]interface{} `yaml:",inline"`
 }
@@ -172,6 +198,12 @@ func (c *HTTPS) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if err := unmarshal((*plain)(c)); err != nil {
 		return err
 	}
+	if c.ReadTimeout == 0 {
+		c.ReadTimeout = Duration(time.Minute)
+	}
+	if c.IdleTimeout == 0 {
+		c.IdleTimeout = Duration(time.Minute * 10)
+	}
 	if len(c.ListenAddr) == 0 {
 		c.ListenAddr = ":443"
 	}
@@ -646,21 +678,45 @@ func LoadFile(filename string) (*Config, error) {
 	if cfg.Server.Metrics.AllowedNetworks, err = cfg.groupToNetwork(cfg.Server.Metrics.NetworksOrGroups); err != nil {
 		return nil, err
 	}
+	var maxResponseTime time.Duration
 	for i := range cfg.Clusters {
 		c := &cfg.Clusters[i]
 		for j := range c.ClusterUsers {
 			u := &c.ClusterUsers[j]
+			cud := time.Duration(u.MaxExecutionTime + u.MaxQueueTime)
+			if cud > maxResponseTime {
+				maxResponseTime = cud
+			}
 			if u.AllowedNetworks, err = cfg.groupToNetwork(u.NetworksOrGroups); err != nil {
 				return nil, err
 			}
 		}
 	}
 	for i := range cfg.Users {
 		u := &cfg.Users[i]
+		ud := time.Duration(u.MaxExecutionTime + u.MaxQueueTime)
+		if ud > maxResponseTime {
+			maxResponseTime = ud
+		}
 		if u.AllowedNetworks, err = cfg.groupToNetwork(u.NetworksOrGroups); err != nil {
 			return nil, err
 		}
 	}
+
+	if maxResponseTime < 0 {
+		maxResponseTime = 0
+	}
+	// Give an additional minute for the maximum response time,
+	// so the response body may be sent to the requester.
+	maxResponseTime += time.Minute
+	if len(cfg.Server.HTTP.ListenAddr) > 0 && cfg.Server.HTTP.WriteTimeout == 0 {
+		cfg.Server.HTTP.WriteTimeout = Duration(maxResponseTime)
+	}
+
+	if len(cfg.Server.HTTPS.ListenAddr) > 0 && cfg.Server.HTTPS.WriteTimeout == 0 {
+		cfg.Server.HTTPS.WriteTimeout = Duration(maxResponseTime)
+	}
+
 	if err := cfg.checkVulnerabilities(); err != nil {
 		return nil, fmt.Errorf("security breach: %s\nSet option `hack_me_please=true` to disable security errors", err)
 	}

config/config_test.go

@@ -39,13 +39,23 @@ func TestLoadConfig(t *testing.T) {
 						ListenAddr:           ":9090",
 						NetworksOrGroups:     []string{"office", "reporting-apps", "1.2.3.4"},
 						ForceAutocertHandler: true,
+						TimeoutCfg: TimeoutCfg{
+							ReadTimeout:  Duration(5 * time.Minute),
+							WriteTimeout: Duration(10 * time.Minute),
+							IdleTimeout:  Duration(20 * time.Minute),
+						},
 					},
 					HTTPS: HTTPS{
 						ListenAddr: ":443",
 						Autocert: Autocert{
 							CacheDir:     "certs_dir",
 							AllowedHosts: []string{"example.com"},
 						},
+						TimeoutCfg: TimeoutCfg{
+							ReadTimeout:  Duration(time.Minute),
+							WriteTimeout: Duration(140 * time.Second),
+							IdleTimeout:  Duration(10 * time.Minute),
+						},
 					},
 					Metrics: Metrics{
 						NetworksOrGroups: []string{"office"},
@@ -196,6 +206,11 @@ func TestLoadConfig(t *testing.T) {
 					HTTP: HTTP{
 						ListenAddr:       ":8080",
 						NetworksOrGroups: []string{"127.0.0.1"},
+						TimeoutCfg: TimeoutCfg{
+							ReadTimeout:  Duration(time.Minute),
+							WriteTimeout: Duration(time.Minute),
+							IdleTimeout:  Duration(10 * time.Minute),
+						},
 					},
 				},
 				Clusters: []Cluster{
@@ -518,3 +533,79 @@ func TestParseDurationNegative(t *testing.T) {
 		}
 	}
 }
+
+func TestConfigTimeouts(t *testing.T) {
+	var testCases = []struct {
+		name        string
+		file        string
+		expectedCfg TimeoutCfg
+	}{
+		{
+			"default",
+			"testdata/default_values.yml",
+			TimeoutCfg{
+				ReadTimeout:  Duration(time.Minute),
+				WriteTimeout: Duration(time.Minute),
+				IdleTimeout:  Duration(10 * time.Minute),
+			},
+		},
+		{
+			"defined",
+			"testdata/timeouts.defined.yml",
+			TimeoutCfg{
+				ReadTimeout:  Duration(time.Minute),
+				WriteTimeout: Duration(time.Hour),
+				IdleTimeout:  Duration(24 * time.Hour),
+			},
+		},
+		{
+			"calculated write 1",
+			"testdata/timeouts.write.calculated.yml",
+			TimeoutCfg{
+				ReadTimeout: Duration(time.Minute),
+				// 10 + 1 minute
+				WriteTimeout: Duration(11 * 60 * time.Second),
+				IdleTimeout:  Duration(10 * time.Minute),
+			},
+		},
+		{
+			"calculated write 2",
+			"testdata/timeouts.write.calculated2.yml",
+			TimeoutCfg{
+				ReadTimeout: Duration(time.Minute),
+				// 20 + 1 minute
+				WriteTimeout: Duration(21 * 60 * time.Second),
+				IdleTimeout:  Duration(10 * time.Minute),
+			},
+		},
+		{
+			"calculated write 3",
+			"testdata/timeouts.write.calculated3.yml",
+			TimeoutCfg{
+				ReadTimeout: Duration(time.Minute),
+				// 50 + 1 minute
+				WriteTimeout: Duration(51 * 60 * time.Second),
+				IdleTimeout:  Duration(10 * time.Minute),
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg, err := LoadFile(tc.file)
+			if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+			got := cfg.Server.HTTP.TimeoutCfg
+			if got.ReadTimeout != tc.expectedCfg.ReadTimeout {
+				t.Fatalf("got ReadTimeout %v; expected to have: %v", got.ReadTimeout, tc.expectedCfg.ReadTimeout)
+			}
+			if got.WriteTimeout != tc.expectedCfg.WriteTimeout {
+				t.Fatalf("got WriteTimeout %v; expected to have: %v", got.WriteTimeout, tc.expectedCfg.WriteTimeout)
+			}
+			if got.IdleTimeout != tc.expectedCfg.IdleTimeout {
+				t.Fatalf("got IdleTimeout %v; expected to have: %v", got.IdleTimeout, tc.expectedCfg.IdleTimeout)
+			}
+		})
+	}
+}

config/testdata/full.yml

@@ -90,6 +90,19 @@ server:
     # By default requests are accepted from all the IPs.
     allowed_networks: ["office", "reporting-apps", "1.2.3.4"]
 
+    # ReadTimeout is the maximum duration for proxy to reading the entire
+    # request, including the body.
+    # Default value is 1m
+    read_timeout: 5m
+
+    # WriteTimeout is the maximum duration for proxy before timing out writes of the response.
+    # Default is largest MaxExecutionTime + MaxQueueTime value from Users or Clusters
+    write_timeout: 10m
+
+    # IdleTimeout is the maximum amount of time for proxy to wait for the next request.
+    # Default is 10m
+    idle_timeout: 20m
+
   # Configs for input https interface.
   # The interface works only if this section is present.
   https:

config/testdata/timeouts.defined.yml

@@ -0,0 +1,16 @@
+hack_me_please: true
+server:
+  http:
+    listen_addr: ":8080"
+    read_timeout: 1m
+    write_timeout: 1h
+    idle_timeout: 1d
+
+users:
+- name: "default"
+  to_cluster: "cluster"
+  to_user: "default"
+
+clusters:
+- name: "cluster"
+  nodes: ["127.0.0.1:8123"]

config/testdata/timeouts.write.calculated.yml

@@ -0,0 +1,17 @@
+hack_me_please: true
+server:
+  http:
+    listen_addr: ":8080"
+
+users:
+- name: "default"
+  to_cluster: "cluster"
+  to_user: "default"
+  max_execution_time: 5m
+
+clusters:
+- name: "cluster"
+  nodes: ["127.0.0.1:8123"]
+  users:
+  - name: "web"
+    max_execution_time: 10m

config/testdata/timeouts.write.calculated2.yml

@@ -0,0 +1,21 @@
+hack_me_please: true
+server:
+  http:
+    listen_addr: ":8080"
+
+users:
+- name: "default"
+  to_cluster: "cluster"
+  to_user: "default"
+  max_execution_time: 5m
+- name: "default2"
+  to_cluster: "cluster"
+  to_user: "default"
+  max_execution_time: 20m
+
+clusters:
+- name: "cluster"
+  nodes: ["127.0.0.1:8123"]
+  users:
+  - name: "web"
+    max_execution_time: 10m

config/testdata/timeouts.write.calculated3.yml

@@ -0,0 +1,23 @@
+hack_me_please: true
+server:
+  http:
+    listen_addr: ":8080"
+
+users:
+- name: "default"
+  to_cluster: "cluster"
+  to_user: "default"
+  max_execution_time: 5m
+- name: "default2"
+  to_cluster: "cluster"
+  to_user: "default"
+  max_execution_time: 20m
+
+clusters:
+- name: "cluster"
+  nodes: ["127.0.0.1:8123"]
+  users:
+  - name: "web"
+    max_execution_time: 10m
+  - name: "web2"
+    max_execution_time: 50m