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) <[email protected]>
openfaas · Sep 1, 2019 · df9701c · df9701c
1 parent 029051b
commit df9701c
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)