From df9701c475bdad9c57494eab72d88b8f3a7a59f1 Mon Sep 17 00:00:00 2001 From: "Alex Ellis (OpenFaaS Ltd)" Date: Sun, 1 Sep 2019 09:07:49 +0100 Subject: [PATCH] Clarify buffer_http A user was confused by what "buffer_http" meant and assumed it meant the response, but the README describes it as being about the request input. This patch was tested with unit tests and makes the value more descriptive to avoid future confusion. Signed-off-by: Alex Ellis (OpenFaaS Ltd) --- README.md | 30 ++++++++++++++++-------------- config/config.go | 17 ++++++++++++++++- config/config_test.go | 30 +++++++++++++++++++++++++++++- main.go | 2 +- 4 files changed, 62 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 31caf87f..92c926d1 100644 --- a/README.md +++ b/README.md @@ -153,19 +153,21 @@ Environmental variables: > Note: timeouts should be specified as Golang durations i.e. `1m` or `20s`. -| Option | Implemented | Usage | -|------------------------|--------------|-------------------------------| -| `function_process` | Yes | Process to execute a server in `http` mode or to be executed for each request in the other modes. For non `http` mode the process must accept input via STDIN and print output via STDOUT. Alias: `fprocess` | -| `static_path` | Yes | Absolute or relative path to the directory that will be served if `mode="static"` | -| `read_timeout` | Yes | HTTP timeout for reading the payload from the client caller (in seconds) | -| `write_timeout` | Yes | HTTP timeout for writing a response body from your function (in seconds) | -| `exec_timeout` | Yes | Exec timeout for process exec'd for each incoming request (in seconds). Disabled if set to 0. | -| `port` | Yes | Specify an alternative TCP port for testing. Default: `8080` | -| `write_debug` | No | Write all output, error messages, and additional information to the logs. Default is `false`. | -| `content_type` | Yes | Force a specific Content-Type response for all responses - only in forking/serializing modes. | -| `suppress_lock` | Yes | When set to `false` the watchdog will attempt to write a lockfile to /tmp/ for healthchecks. Default `false` | -| `upstream_url` | Yes | `http` mode only - where to forward requests i.e. `127.0.0.1:5000` | -| `buffer_http` | Yes | `http` mode only - buffers request body to memory before forwarding. Use if your upstream HTTP server does not accept `Transfer-Encoding: chunked` Default: `false` | -| `max_inflight` | Yes | Limit the maximum number of requests in flight | +| Option | Implemented | Usage | +|-----------------------------|--------------|-------------------------------| +| `function_process` | Yes | Process to execute a server in `http` mode or to be executed for each request in the other modes. For non `http` mode the process must accept input via STDIN and print output via STDOUT. Alias: `fprocess` | +| `static_path` | Yes | Absolute or relative path to the directory that will be served if `mode="static"` | +| `read_timeout` | Yes | HTTP timeout for reading the payload from the client caller (in seconds) | +| `write_timeout` | Yes | HTTP timeout for writing a response body from your function (in seconds) | +| `exec_timeout` | Yes | Exec timeout for process exec'd for each incoming request (in seconds). Disabled if set to 0. | +| `port` | Yes | Specify an alternative TCP port for testing. Default: `8080` | +| `write_debug` | No | Write all output, error messages, and additional information to the logs. Default is `false`. | +| `content_type` | Yes | Force a specific Content-Type response for all responses - only in forking/serializing modes. | +| `suppress_lock` | Yes | When set to `false` the watchdog will attempt to write a lockfile to /tmp/ for healthchecks. Default `false` | +| `http_upstream_url` | Yes | `http` mode only - where to forward requests i.e. `127.0.0.1:5000` | +| `upstream_url` | Yes | alias for `http_upstream_url` | +| `http_buffer_req_body` | Yes | `http` mode only - buffers request body in memory before forwarding upstream to your template's `upstream_url`. Use if your upstream HTTP server does not accept `Transfer-Encoding: chunked` Default: `false` | +| `buffer_http` | Yes | deprecated alias for `http_buffer_req_body`, will be removed in future version | +| `max_inflight` | Yes | Limit the maximum number of requests in flight | > Note: the .lock file is implemented for health-checking, but cannot be disabled yet. You must create this file in /tmp/. diff --git a/config/config.go b/config/config.go index f035315a..e660df0d 100644 --- a/config/config.go +++ b/config/config.go @@ -70,6 +70,10 @@ func New(env []string) WatchdogConfig { upstreamURL = val } + if val, exists := envMap["http_upstream_url"]; exists { + upstreamURL = val + } + contentType := "application/octet-stream" if val, exists := envMap["content_type"]; exists { contentType = val @@ -92,7 +96,7 @@ func New(env []string) WatchdogConfig { ContentType: contentType, SuppressLock: getBool(envMap, "suppress_lock"), UpstreamURL: upstreamURL, - BufferHTTPBody: getBool(envMap, "buffer_http"), + BufferHTTPBody: getBools(envMap, "buffer_http", "http_buffer_req_body"), MetricsPort: 8081, MaxInflight: getInt(envMap, "max_inflight", 0), } @@ -151,3 +155,14 @@ func getBool(env map[string]string, key string) bool { return false } + +func getBools(env map[string]string, key ...string) bool { + v := false + for _, k := range key { + if getBool(env, k) == true { + v = true + break + } + } + return v +} diff --git a/config/config_test.go b/config/config_test.go index d3f30528..eeec16bb 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "testing" "time" ) @@ -18,6 +19,7 @@ func Test_OperationalMode_Default(t *testing.T) { t.Errorf("Want %s. got: %s", WatchdogMode(ModeStreaming), WatchdogMode(defaults.OperationalMode)) } } + func Test_BufferHttpModeDefaultsToFalse(t *testing.T) { env := []string{} @@ -28,9 +30,35 @@ func Test_BufferHttpModeDefaultsToFalse(t *testing.T) { } } +func Test_UpstreamURL(t *testing.T) { + urlVal := "http://127.0.0.1:8082" + env := []string{ + fmt.Sprintf("upstream_url=%s", urlVal), + } + + actual := New(env) + want := urlVal + if actual.UpstreamURL != want { + t.Errorf("Want %v. got: %v", want, actual.UpstreamURL) + } +} + +func Test_UpstreamURLVerbose(t *testing.T) { + urlVal := "http://127.0.0.1:8082" + env := []string{ + fmt.Sprintf("http_upstream_url=%s", urlVal), + } + + actual := New(env) + want := urlVal + if actual.UpstreamURL != want { + t.Errorf("Want %v. got: %v", want, actual.UpstreamURL) + } +} + func Test_BufferHttpMode_CanBeSetToTrue(t *testing.T) { env := []string{ - "buffer_http=true", + "http_buffer_req_body=true", } actual := New(env) diff --git a/main.go b/main.go index c7efb5b8..1870019b 100644 --- a/main.go +++ b/main.go @@ -301,7 +301,7 @@ func makeHTTPRequestHandler(watchdogConfig config.WatchdogConfig) func(http.Resp } if len(watchdogConfig.UpstreamURL) == 0 { - log.Fatal(`For mode=http you must specify a valid URL for "upstream_url"`) + log.Fatal(`For "mode=http" you must specify a valid URL for "http_upstream_url"`) } urlValue, upstreamURLErr := url.Parse(watchdogConfig.UpstreamURL)