From 95ec131fa2e43db229d7964a6dcd1ba3b258224d Mon Sep 17 00:00:00 2001 From: Lexman42 Date: Wed, 1 May 2019 13:41:26 -0700 Subject: [PATCH 1/4] http timeout fields are configurable --- command/server.go | 53 ++++++++++++++++++++++++++++++ command/server_test.go | 73 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/command/server.go b/command/server.go index 50de4462c9ea..6b2fe62226ed 100644 --- a/command/server.go +++ b/command/server.go @@ -106,6 +106,7 @@ type ServerCommand struct { flagDevAutoSeal bool flagTestVerifyOnly bool flagCombineLogs bool + flagTestServerConfig bool } type ServerListener struct { @@ -305,6 +306,13 @@ func (c *ServerCommand) Flags() *FlagSets { Hidden: true, }) + f.BoolVar(&BoolVar{ + Name: "test-server-config", + Target: &c.flagTestServerConfig, + Default: false, + Hidden: true, + }) + // End internal-only flags. return set @@ -1126,6 +1134,7 @@ CLUSTER_SYNTHESIS_COMPLETE: c.UI.Warn("") } + // Continuing configuration testing at this point // Initialize the HTTP servers for _, ln := range lns { handler := vaulthttp.Handler(&vault.HandlerProperties{ @@ -1146,6 +1155,7 @@ CLUSTER_SYNTHESIS_COMPLETE: } } + // server defaults server := &http.Server{ Handler: handler, ReadHeaderTimeout: 10 * time.Second, @@ -1153,6 +1163,49 @@ CLUSTER_SYNTHESIS_COMPLETE: IdleTimeout: 5 * time.Minute, ErrorLog: c.logger.StandardLogger(nil), } + + // override server defaults with config values for read/write/idle timeouts if configured + if readHeaderTimeoutInterface, ok := ln.config["http_read_header_timeout"]; ok { + readHeaderTimeout, err := parseutil.ParseDurationSecond(readHeaderTimeoutInterface) + if err != nil { + c.UI.Error(fmt.Sprintf("Could not parse a time value for http_read_header_timeout %v", readHeaderTimeout)) + return 1 + } + server.ReadHeaderTimeout = readHeaderTimeout + } + + if readTimeoutInterface, ok := ln.config["http_read_timeout"]; ok { + readTimeout, err := parseutil.ParseDurationSecond(readTimeoutInterface) + if err != nil { + c.UI.Error(fmt.Sprintf("Could not parse a time value for http_read_timeout %v", readTimeout)) + return 1 + } + server.ReadTimeout = readTimeout + } + + if writeTimeoutInterface, ok := ln.config["http_write_timeout"]; ok { + writeTimeout, err := parseutil.ParseDurationSecond(writeTimeoutInterface) + if err != nil { + c.UI.Error(fmt.Sprintf("Could not parse a time value for http_write_timeout %v", writeTimeout)) + return 1 + } + server.WriteTimeout = writeTimeout + } + + if idleTimeoutInterface, ok := ln.config["http_idle_timeout"]; ok { + idleTimeout, err := parseutil.ParseDurationSecond(idleTimeoutInterface) + if err != nil { + c.UI.Error(fmt.Sprintf("Could not parse a time value for http_idle_timeout %v", idleTimeout)) + return 1 + } + server.IdleTimeout = idleTimeout + } + + // server config tests can exit now + if c.flagTestServerConfig { + return 0 + } + go server.Serve(ln.Listener) } diff --git a/command/server_test.go b/command/server_test.go index 6bdf21fc5593..c6396fffa869 100644 --- a/command/server_test.go +++ b/command/server_test.go @@ -41,19 +41,30 @@ func testRandomPort(tb testing.TB) int { return l.Addr().(*net.TCPAddr).Port } -func testBaseHCL(tb testing.TB) string { +func testBaseHCL(tb testing.TB, listenerExtras string) string { tb.Helper() return strings.TrimSpace(fmt.Sprintf(` disable_mlock = true listener "tcp" { - address = "127.0.0.1:%d" - tls_disable = "true" + address = "127.0.0.1:%d" + tls_disable = "true" + %s } - `, testRandomPort(tb))) + `, testRandomPort(tb), listenerExtras)) } const ( + goodListenerTimeouts = `http_read_header_timeout = 12 + http_read_timeout = "34s" + http_write_timeout = "56m" + http_idle_timeout = "78h"` + + badListenerReadHeaderTimeout = `http_read_header_timeout = "12km"` + badListenerReadTimeout = `http_read_timeout = "34日"` + badListenerWriteTimeout = `http_write_timeout = "56lbs"` + badListenerIdleTimeout = `http_idle_timeout = "78gophers"` + inmemHCL = ` backend "inmem_ha" { advertise_addr = "http://127.0.0.1:8200" @@ -204,24 +215,70 @@ func TestServer(t *testing.T) { contents string exp string code int + flag string }{ { "common_ha", - testBaseHCL(t) + inmemHCL, + testBaseHCL(t, "") + inmemHCL, "(HA available)", 0, + "-test-verify-only", }, { "separate_ha", - testBaseHCL(t) + inmemHCL + haInmemHCL, + testBaseHCL(t, "") + inmemHCL + haInmemHCL, "HA Storage:", 0, + "-test-verify-only", }, { "bad_separate_ha", - testBaseHCL(t) + inmemHCL + badHAInmemHCL, + testBaseHCL(t, "") + inmemHCL + badHAInmemHCL, "Specified HA storage does not support HA", 1, + "-test-verify-only", + }, + { + "good_listener_timeout_config", + testBaseHCL(t, goodListenerTimeouts) + inmemHCL, + "", + 0, + "-test-server-config", + }, + { + "bad_listener_read_header_timeout_config", + testBaseHCL(t, badListenerReadHeaderTimeout) + inmemHCL, + "Could not parse a time value for http_read_header_timeout", + 1, + "-test-server-config", + }, + { + "bad_listener_read_header_timeout_config", + testBaseHCL(t, badListenerReadHeaderTimeout) + inmemHCL, + "Could not parse a time value for http_read_header_timeout", + 1, + "-test-server-config", + }, + { + "bad_listener_read_timeout_config", + testBaseHCL(t, badListenerReadTimeout) + inmemHCL, + "Could not parse a time value for http_read_timeout", + 1, + "-test-server-config", + }, + { + "bad_listener_write_timeout_config", + testBaseHCL(t, badListenerWriteTimeout) + inmemHCL, + "Could not parse a time value for http_write_timeout", + 1, + "-test-server-config", + }, + { + "bad_listener_idle_timeout_config", + testBaseHCL(t, badListenerIdleTimeout) + inmemHCL, + "Could not parse a time value for http_idle_timeout", + 1, + "-test-server-config", }, } @@ -242,7 +299,7 @@ func TestServer(t *testing.T) { code := cmd.Run([]string{ "-config", f.Name(), - "-test-verify-only", + tc.flag, }) output := ui.ErrorWriter.String() + ui.OutputWriter.String() if code != tc.code { From 4ac7a85c3b89906699944e17ca778820421edf43 Mon Sep 17 00:00:00 2001 From: Lexman42 Date: Mon, 6 May 2019 09:10:19 -0700 Subject: [PATCH 2/4] move return statement for server config tests outside of range loop --- command/server.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/command/server.go b/command/server.go index 6b2fe62226ed..b97ba1422326 100644 --- a/command/server.go +++ b/command/server.go @@ -1134,7 +1134,6 @@ CLUSTER_SYNTHESIS_COMPLETE: c.UI.Warn("") } - // Continuing configuration testing at this point // Initialize the HTTP servers for _, ln := range lns { handler := vaulthttp.Handler(&vault.HandlerProperties{ @@ -1203,12 +1202,16 @@ CLUSTER_SYNTHESIS_COMPLETE: // server config tests can exit now if c.flagTestServerConfig { - return 0 + continue } go server.Serve(ln.Listener) } + if c.flagTestServerConfig { + return 0 + } + if sealConfigError != nil { init, err := core.Initialized(context.Background()) if err != nil { From 3b0901f751f54dc4ae6fda72a38713888b559f36 Mon Sep 17 00:00:00 2001 From: Lexman42 Date: Fri, 10 May 2019 10:08:48 -0700 Subject: [PATCH 3/4] adds documentation for configurable listener http_* values --- .../docs/configuration/listener/tcp.html.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/website/source/docs/configuration/listener/tcp.html.md b/website/source/docs/configuration/listener/tcp.html.md index e301ece6a99a..9f6a8f062244 100644 --- a/website/source/docs/configuration/listener/tcp.html.md +++ b/website/source/docs/configuration/listener/tcp.html.md @@ -35,6 +35,24 @@ advertise the correct address to other nodes. they need to hop through a TCP load balancer or some other scheme in order to talk. +- `http_idle_timeout` `(string: "5m")` - Specifies the maximum amount of time to + wait for the next request when keep-alives are enabled. If `http_idle_timeout` + is zero, the value of `http_read_timeout` is used. If both are zero, the value of `http_read_header_timeout` is used. This is specified using a label suffix like + `"30s"` or `"1h"`. + +- `http_read_header_timeout` `(string: "10s")` - Specifies the amount of time + allowed to read request headers. This is specified using a label suffix like + `"30s"` or `"1h"`. + +- `http_read_timeout` `(string: "30s")` - Specifies the maximum duration for + reading the entire request, including the body. This is specified using a + label suffix like `"30s"` or `"1h"`. + +- `http_write_timeout` `string: "0")` - Specifies the maximum duration before + timing out writes of the response and is reset whenever a new request's header + is read. The default value of `"0"` means inifinity. This is specified using a + label suffix like `"30s"` or `"1h"`. + - `max_request_size` `(int: 33554432)` – Specifies a hard maximum allowed request size, in bytes. Defaults to 32 MB. Specifying a number less than or equal to `0` turns off limiting altogether. From 637a58f25c7de74635c36e64e46fba0df1feaf17 Mon Sep 17 00:00:00 2001 From: Lexman42 Date: Fri, 10 May 2019 10:10:23 -0700 Subject: [PATCH 4/4] fixed some formatting for the docs markdown --- website/source/docs/configuration/listener/tcp.html.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/website/source/docs/configuration/listener/tcp.html.md b/website/source/docs/configuration/listener/tcp.html.md index 9f6a8f062244..d8e0297a910f 100644 --- a/website/source/docs/configuration/listener/tcp.html.md +++ b/website/source/docs/configuration/listener/tcp.html.md @@ -37,8 +37,9 @@ advertise the correct address to other nodes. - `http_idle_timeout` `(string: "5m")` - Specifies the maximum amount of time to wait for the next request when keep-alives are enabled. If `http_idle_timeout` - is zero, the value of `http_read_timeout` is used. If both are zero, the value of `http_read_header_timeout` is used. This is specified using a label suffix like - `"30s"` or `"1h"`. + is zero, the value of `http_read_timeout` is used. If both are zero, the value + of `http_read_header_timeout` is used. This is specified using a label suffix + like `"30s"` or `"1h"`. - `http_read_header_timeout` `(string: "10s")` - Specifies the amount of time allowed to read request headers. This is specified using a label suffix like