Repository: buchgr/bazel-remote
Pull request #287 (2020-06-03 16:35:24)

land @elianuber's http timeout flags

mostynb: This is a squash+rebase of #271.

Patch:
diff --git a/README.md b/README.md
index 6e93b8f..72895e8 100644
--- a/README.md
+++ b/README.md
@@ -112,6 +112,8 @@ GLOBAL OPTIONS:
    --grpc_port value                The port the gRPC server listens on. Set to 0 to disable. (default: 9092) [$BAZEL_REMOTE_GRPC_PORT]
    --profile_host value             A host address to listen on for profiling, if enabled by a valid --profile_port setting. (default: "127.0.0.1") [$BAZEL_REMOTE_PROFILE_HOST]
    --profile_port value             If a positive integer, serve /debug/pprof/* URLs from http://profile_host:profile_port. (default: 0, ie profiling disabled) [$BAZEL_REMOTE_PROFILE_PORT]
+   --http_read_timeout value        The HTTP read timeout for a client request in seconds (does not apply to the proxy backends or the profiling endpoint) (default: 0s, ie disabled) [$BAZEL_REMOTE_HTTP_READ_TIMEOUT]
+   --http_write_timeout value       The HTTP write timeout for a server response in seconds (does not apply to the proxy backends or the profiling endpoint) (default: 0s, ie disabled) [$BAZEL_REMOTE_HTTP_WRITE_TIMEOUT]
    --htpasswd_file value            Path to a .htpasswd file. This flag is optional. Please read https://httpd.apache.org/docs/2.4/programs/htpasswd.html. [$BAZEL_REMOTE_HTPASSWD_FILE]
    --tls_enabled                    This flag has been deprecated. Specify tls_cert_file and tls_key_file instead. (default: false) [$BAZEL_REMOTE_TLS_ENABLED]
    --tls_cert_file value            Path to a pem encoded certificate file. [$BAZEL_REMOTE_TLS_CERT_FILE]
@@ -149,6 +151,12 @@ host: localhost
 #profile_host: 127.0.0.1
 #profile_port: 7070
 
+# HTTP read/write timeouts.
+# (does not apply to the proxy backends or the profiling endpoint)
+# Time units can be one of: "s", "m", "h".
+#http_read_timeout: 5s
+#http_write_timeout: 10s
+
 # If you want to require simple authentication:
 #htpasswd_file: path/to/.htpasswd
 
diff --git a/config/config.go b/config/config.go
index 21d2052..ca39ae7 100644
--- a/config/config.go
+++ b/config/config.go
@@ -54,6 +54,8 @@ type Config struct {
 	DisableGRPCACDepsCheck     bool                      `yaml:"disable_grpc_ac_deps_check"`
 	EnableEndpointMetrics      bool                      `yaml:"enable_endpoint_metrics"`
 	ExperimentalRemoteAssetAPI bool                      `yaml:"experimental_remote_asset_api"`
+	HTTPReadTimeout            time.Duration             `yaml:"http_read_timeout"`
+	HTTPWriteTimeout           time.Duration             `yaml:"http_write_timeout"`
 }
 
 // New returns a validated Config with the specified values, and an error
@@ -63,7 +65,8 @@ func New(dir string, maxSize int, host string, port int, grpcPort int,
 	tlsCertFile string, tlsKeyFile string, idleTimeout time.Duration,
 	s3 *S3CloudStorageConfig, disableHTTPACValidation bool,
 	disableGRPCACDepsCheck bool, enableEndpointMetrics bool,
-	experimentalRemoteAssetAPI bool) (*Config, error) {
+	experimentalRemoteAssetAPI bool,
+	httpReadTimeout time.Duration, httpWriteTimeout time.Duration) (*Config, error) {
 	c := Config{
 		Host:                       host,
 		Port:                       port,
@@ -83,6 +86,8 @@ func New(dir string, maxSize int, host string, port int, grpcPort int,
 		DisableGRPCACDepsCheck:     disableGRPCACDepsCheck,
 		EnableEndpointMetrics:      enableEndpointMetrics,
 		ExperimentalRemoteAssetAPI: experimentalRemoteAssetAPI,
+		HTTPReadTimeout:            httpReadTimeout,
+		HTTPWriteTimeout:           httpWriteTimeout,
 	}
 
 	err := validateConfig(&c)
diff --git a/main.go b/main.go
index 1e147d4..4a225bd 100644
--- a/main.go
+++ b/main.go
@@ -112,6 +112,20 @@ func main() {
 			DefaultText: "0, ie profiling disabled",
 			EnvVars:     []string{"BAZEL_REMOTE_PROFILE_PORT"},
 		},
+		&cli.DurationFlag{
+			Name:        "http_read_timeout",
+			Value:       0,
+			Usage:       "The HTTP read timeout for a client request in seconds (does not apply to the proxy backends or the profiling endpoint)",
+			DefaultText: "0s, ie disabled",
+			EnvVars:     []string{"BAZEL_REMOTE_HTTP_READ_TIMEOUT"},
+		},
+		&cli.DurationFlag{
+			Name:        "http_write_timeout",
+			Value:       0,
+			Usage:       "The HTTP write timeout for a server response in seconds (does not apply to the proxy backends or the profiling endpoint)",
+			DefaultText: "0s, ie disabled",
+			EnvVars:     []string{"BAZEL_REMOTE_HTTP_WRITE_TIMEOUT"},
+		},
 		&cli.StringFlag{
 			Name:    "htpasswd_file",
 			Value:   "",
@@ -253,6 +267,8 @@ func main() {
 				ctx.Bool("disable_grpc_ac_deps_check"),
 				ctx.Bool("enable_endpoint_metrics"),
 				ctx.Bool("experimental_remote_asset_api"),
+				ctx.Duration("http_read_timeout"),
+				ctx.Duration("http_write_timeout"),
 			)
 		}
 
@@ -304,8 +320,10 @@ func main() {
 
 		mux := http.NewServeMux()
 		httpServer := &http.Server{
-			Addr:    c.Host + ":" + strconv.Itoa(c.Port),
-			Handler: mux,
+			Addr:         c.Host + ":" + strconv.Itoa(c.Port),
+			Handler:      mux,
+			ReadTimeout:  c.HTTPReadTimeout,
+			WriteTimeout: c.HTTPWriteTimeout,
 		}
 
 		validateAC := !c.DisableHTTPACValidation


Test patch:
diff --git a/config/config_test.go b/config/config_test.go
index d7feef3..1e7419c 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -4,6 +4,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 )
@@ -20,6 +21,8 @@ tls_key_file:  /opt/tls.key
 disable_http_ac_validation: true
 enable_endpoint_metrics: true
 experimental_remote_asset_api: true
+http_read_timeout: 5s
+http_write_timeout: 10s
 `
 
 	config, err := newFromYaml([]byte(yaml))
@@ -39,6 +42,8 @@ experimental_remote_asset_api: true
 		DisableHTTPACValidation:    true,
 		EnableEndpointMetrics:      true,
 		ExperimentalRemoteAssetAPI: true,
+		HTTPReadTimeout:            5 * time.Second,
+		HTTPWriteTimeout:           10 * time.Second,
 	}
 
 	if !reflect.DeepEqual(config, expectedConfig) {