Introduce Options for the httpclient

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

changelog/unreleased/make-httpclient-configurable.md

@@ -0,0 +1,5 @@
+Enhancement: Make httpclient configurable
+
+- Introduce Options for the httpclient (#914)
+
+https://github.com/cs3org/reva/pull/914

cmd/reva/download.go

@@ -23,6 +23,7 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"time"
 
 	"github.com/cs3org/reva/internal/http/services/datagateway"
 
@@ -92,7 +93,13 @@ func downloadCommand() *command {
 		}
 
 		httpReq.Header.Set(datagateway.TokenTransportHeader, res.Token)
-		httpClient := rhttp.GetHTTPClient(ctx)
+		httpClient := rhttp.GetHTTPClient(
+			rhttp.Context(ctx),
+			// TODO make insecure configurable
+			rhttp.Insecure(true),
+			// TODO make timeout configurable
+			rhttp.Timeout(time.Duration(24*int64(time.Hour))),
+		)
 
 		httpRes, err := httpClient.Do(httpReq)
 		if err != nil {

cmd/reva/upload.go

@@ -25,6 +25,7 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"time"
 
 	"github.com/cs3org/reva/internal/http/services/datagateway"
 
@@ -129,7 +130,13 @@ func uploadCommand() *command {
 		// create the tus client.
 		c := tus.DefaultConfig()
 		c.Resume = true
-		c.HttpClient = rhttp.GetHTTPClient(ctx)
+		c.HttpClient = rhttp.GetHTTPClient(
+			rhttp.Context(ctx),
+			// TODO make insecure configurable
+			rhttp.Insecure(true),
+			// TODO make timeout configurable
+			rhttp.Timeout(time.Duration(24*int64(time.Hour))),
+		)
 		c.Store, err = memorystore.NewMemoryStore()
 		if err != nil {
 			return err

examples/meshdirectory/meshdirectory.toml

@@ -13,3 +13,5 @@ driver = "mentix"
 #
 #[http.services.meshdirectory.drivers.mentix]
 #url = "http://localhost:9600/"
+#insecure = true
+#timeout = 10

internal/http/services/datagateway/datagateway.go

@@ -24,6 +24,7 @@ import (
 	"net/http"
 	"net/url"
 	"path"
+	"time"
 
 	"github.com/cs3org/reva/pkg/appctx"
 	"github.com/cs3org/reva/pkg/errtypes"
@@ -53,6 +54,8 @@ type transferClaims struct {
 type config struct {
 	Prefix               string `mapstructure:"prefix"`
 	TransferSharedSecret string `mapstructure:"transfer_shared_secret"`
+	Timeout              int64  `mapstructure:"timeout"`
+	Insecure             bool   `mapstructure:"insecure"`
 }
 
 func (c *config) init() {
@@ -167,7 +170,11 @@ func (s *svc) doHead(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(ctx)
+	httpClient := rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.conf.Insecure),
+	)
 	httpReq, err := rhttp.NewRequest(ctx, "HEAD", claims.Target, nil)
 	if err != nil {
 		log.Err(err).Msg("wrong request")
@@ -205,7 +212,11 @@ func (s *svc) doGet(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(ctx)
+	httpClient := rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.conf.Insecure),
+	)
 	httpReq, err := rhttp.NewRequest(ctx, "GET", claims.Target, nil)
 	if err != nil {
 		log.Err(err).Msg("wrong request")
@@ -259,7 +270,11 @@ func (s *svc) doPut(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(ctx)
+	httpClient := rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.conf.Insecure),
+	)
 	httpReq, err := rhttp.NewRequest(ctx, "PUT", target, r.Body)
 	if err != nil {
 		log.Err(err).Msg("wrong request")
@@ -314,7 +329,11 @@ func (s *svc) doPatch(w http.ResponseWriter, r *http.Request) {
 
 	log.Debug().Str("target", claims.Target).Msg("sending request to internal data server")
 
-	httpClient := rhttp.GetHTTPClient(ctx)
+	httpClient := rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.conf.Insecure),
+	)
 	httpReq, err := rhttp.NewRequest(ctx, "PATCH", target, r.Body)
 	if err != nil {
 		log.Err(err).Msg("wrong request")

internal/http/services/dataprovider/dataprovider.go

@@ -40,6 +40,8 @@ type config struct {
 	Driver     string                            `mapstructure:"driver" docs:"localhome;The storage driver to be used."`
 	Drivers    map[string]map[string]interface{} `mapstructure:"drivers" docs:"url:docs/config/packages/storage/fs;The configuration for the storage driver"`
 	DisableTus bool                              `mapstructure:"disable_tus" docs:"false;Whether to disable TUS uploads."`
+	Timeout    int64                             `mapstructure:"timeout"`
+	Insecure   bool                              `mapstructure:"insecure"`
 }
 
 func (c *config) init() {

internal/http/services/dataprovider/put.go

@@ -24,6 +24,7 @@ import (
 	"path"
 	"strconv"
 	"strings"
+	"time"
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/pkg/appctx"
@@ -73,7 +74,11 @@ func (s *svc) doTusPut(w http.ResponseWriter, r *http.Request) {
 	// create the tus client.
 	c := tus.DefaultConfig()
 	c.Resume = true
-	c.HttpClient = rhttp.GetHTTPClient(ctx)
+	c.HttpClient = rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.conf.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.conf.Insecure),
+	)
 	c.Store, err = memorystore.NewMemoryStore()
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)

internal/http/services/owncloud/ocdav/copy.go

@@ -26,6 +26,7 @@ import (
 	"net/url"
 	"path"
 	"strings"
+	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
@@ -169,7 +170,7 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) {
 		// TODO what if intermediate is a file?
 	}
 
-	err = descend(ctx, client, srcStatRes.Info, dst, depth == "infinity")
+	err = s.descend(ctx, client, srcStatRes.Info, dst, depth == "infinity")
 	if err != nil {
 		log.Error().Err(err).Msg("error descending directory")
 		w.WriteHeader(http.StatusInternalServerError)
@@ -178,7 +179,7 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) {
 	w.WriteHeader(successCode)
 }
 
-func descend(ctx context.Context, client gateway.GatewayAPIClient, src *provider.ResourceInfo, dst string, recurse bool) error {
+func (s *svc) descend(ctx context.Context, client gateway.GatewayAPIClient, src *provider.ResourceInfo, dst string, recurse bool) error {
 	log := appctx.GetLogger(ctx)
 	log.Debug().Str("src", src.Path).Str("dst", dst).Msg("descending")
 	if src.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
@@ -215,7 +216,7 @@ func descend(ctx context.Context, client gateway.GatewayAPIClient, src *provider
 
 		for i := range res.Infos {
 			childDst := path.Join(dst, path.Base(res.Infos[i].Path))
-			err := descend(ctx, client, res.Infos[i], childDst, recurse)
+			err := s.descend(ctx, client, res.Infos[i], childDst, recurse)
 			if err != nil {
 				return err
 			}
@@ -274,7 +275,11 @@ func descend(ctx context.Context, client gateway.GatewayAPIClient, src *provider
 		}
 		httpDownloadReq.Header.Set(datagateway.TokenTransportHeader, dRes.Token)
 
-		httpDownloadClient := rhttp.GetHTTPClient(ctx)
+		httpDownloadClient := rhttp.GetHTTPClient(
+			rhttp.Context(ctx),
+			rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
+			rhttp.Insecure(s.c.Insecure),
+		)
 
 		httpDownloadRes, err := httpDownloadClient.Do(httpDownloadReq)
 		if err != nil {
@@ -287,22 +292,26 @@ func descend(ctx context.Context, client gateway.GatewayAPIClient, src *provider
 		}
 
 		// do upload
-		err = tusUpload(ctx, uRes.UploadEndpoint, uRes.Token, dst, httpDownloadRes.Body, src.GetSize())
+		err = s.tusUpload(ctx, uRes.UploadEndpoint, uRes.Token, dst, httpDownloadRes.Body, src.GetSize())
 		if err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func tusUpload(ctx context.Context, dataServerURL string, transferToken string, fn string, body io.Reader, length uint64) error {
+func (s *svc) tusUpload(ctx context.Context, dataServerURL string, transferToken string, fn string, body io.Reader, length uint64) error {
 	var err error
 	log := appctx.GetLogger(ctx)
 
 	// create the tus client.
 	c := tus.DefaultConfig()
 	c.Resume = true
-	c.HttpClient = rhttp.GetHTTPClient(ctx)
+	c.HttpClient = rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.c.Insecure),
+	)
 	c.Store, err = memorystore.NewMemoryStore()
 	if err != nil {
 		return err

internal/http/services/owncloud/ocdav/get.go

@@ -106,7 +106,11 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) {
 		return
 	}
 	httpReq.Header.Set(datagateway.TokenTransportHeader, dRes.Token)
-	httpClient := rhttp.GetHTTPClient(ctx)
+	httpClient := rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.c.Insecure),
+	)
 
 	httpRes, err := httpClient.Do(httpReq)
 	if err != nil {

internal/http/services/owncloud/ocdav/ocdav.go

@@ -66,6 +66,8 @@ type Config struct {
 	ChunkFolder     string `mapstructure:"chunk_folder"`
 	GatewaySvc      string `mapstructure:"gatewaysvc"`
 	DisableTus      bool   `mapstructure:"disable_tus"`
+	Timeout         int64  `mapstructure:"timeout"`
+	Insecure        bool   `mapstructure:"insecure"`
 }
 
 func (c *Config) init() {

internal/http/services/owncloud/ocdav/put.go

@@ -253,7 +253,11 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) {
 	// create the tus client.
 	c := tus.DefaultConfig()
 	c.Resume = true
-	c.HttpClient = rhttp.GetHTTPClient(ctx)
+	c.HttpClient = rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
+		rhttp.Insecure(s.c.Insecure),
+	)
 	c.Store, err = memorystore.NewMemoryStore()
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)

internal/http/services/owncloud/ocdav/tus.go

@@ -167,7 +167,11 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) {
 	// TODO check this really streams
 	if r.Header.Get("Content-Type") == "application/offset+octet-stream" {
 
-		httpClient := rhttp.GetHTTPClient(ctx)
+		httpClient := rhttp.GetHTTPClient(
+			rhttp.Context(ctx),
+			rhttp.Timeout(time.Duration(s.c.Timeout*int64(time.Second))),
+			rhttp.Insecure(s.c.Insecure),
+		)
 		httpReq, err := rhttp.NewRequest(ctx, "PATCH", uRes.UploadEndpoint, r.Body)
 		if err != nil {
 			log.Err(err).Msg("wrong request")

pkg/auth/manager/oidc/oidc.go

@@ -22,15 +22,14 @@ package oidc
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
-	"net/http"
 	"time"
 
 	oidc "github.com/coreos/go-oidc"
 	user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	"github.com/cs3org/reva/pkg/auth"
 	"github.com/cs3org/reva/pkg/auth/manager/registry"
+	"github.com/cs3org/reva/pkg/rhttp"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog/log"
@@ -143,18 +142,13 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 func (am *mgr) getOAuthCtx(ctx context.Context) context.Context {
 	// Sometimes for testing we need to skip the TLS check, that's why we need a
 	// custom HTTP client.
-	tr := &http.Transport{
-		TLSClientConfig: &tls.Config{
-			InsecureSkipVerify: am.c.Insecure,
-		},
+	customHTTPClient := rhttp.GetHTTPClient(
+		rhttp.Context(ctx),
+		rhttp.Timeout(time.Second*10),
+		rhttp.Insecure(am.c.Insecure),
 		// Fixes connection fd leak which might be caused by provider-caching
-		DisableKeepAlives: true,
-	}
-
-	customHTTPClient := &http.Client{
-		Transport: tr,
-		Timeout:   time.Second * 10,
-	}
+		rhttp.DisableKeepAlive(true),
+	)
 	ctx = context.WithValue(ctx, oauth2.HTTPClient, customHTTPClient)
 	return ctx
 }

pkg/meshdirectory/manager/mentix/mentix.go

@@ -23,6 +23,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"time"
 
 	"github.com/cs3org/reva/pkg/meshdirectory"
 	"github.com/cs3org/reva/pkg/meshdirectory/manager/registry"
@@ -55,8 +56,12 @@ func New(m map[string]interface{}) (meshdirectory.Manager, error) {
 
 	client := &Client{
 		BaseURL: c.URL,
-		// TODO: pass/create context once it is required by GetHTTPClient
-		HTTPClient: rhttp.GetHTTPClient(context.TODO()),
+		HTTPClient: rhttp.GetHTTPClient(
+			// TODO: pass/create context once it is required by GetHTTPClient
+			rhttp.Context(context.TODO()),
+			rhttp.Insecure(c.Insecure),
+			rhttp.Timeout(time.Duration(c.Timeout*int64(time.Second))),
+		),
 	}
 
 	mgr := &mgr{
@@ -68,7 +73,9 @@ func New(m map[string]interface{}) (meshdirectory.Manager, error) {
 }
 
 type config struct {
-	URL string `mapstructure:"url"`
+	URL      string `mapstructure:"url"`
+	Timeout  int64  `mapstructure:"timeout"`
+	Insecure bool   `mapstructure:"insecure"`
 }
 
 type mgr struct {

pkg/rhttp/client.go

@@ -23,7 +23,6 @@ import (
 	"crypto/tls"
 	"io"
 	"net/http"
-	"time"
 
 	"github.com/cs3org/reva/pkg/token"
 	"github.com/pkg/errors"
@@ -33,15 +32,17 @@ import (
 // GetHTTPClient returns an http client with open census tracing support.
 // TODO(labkode): harden it.
 // https://medium.com/@nate510/don-t-use-go-s-default-http-client-4804cb19f779
-func GetHTTPClient(ctx context.Context) *http.Client {
+func GetHTTPClient(opts ...Option) *http.Client {
+	options := newOptions(opts...)
+
 	httpClient := &http.Client{
-		Timeout: time.Second * 10,
+		Timeout: options.Timeout,
 		Transport: &ochttp.Transport{
 			Base: &http.Transport{
-				// TODO: make TLS config configurable
 				TLSClientConfig: &tls.Config{
-					InsecureSkipVerify: true,
+					InsecureSkipVerify: options.Insecure,
 				},
+				DisableKeepAlives: options.DisableKeepAlive,
 			},
 		},
 	}