diff --git a/changelog/unreleased/.keep b/changelog/unreleased/.keep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/changelog/unreleased/make-httpclient-configurable.md b/changelog/unreleased/make-httpclient-configurable.md new file mode 100644 index 00000000000..fd83b1c494e --- /dev/null +++ b/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 diff --git a/cmd/reva/download.go b/cmd/reva/download.go index d4d4a05f6e0..54533ed9648 100644 --- a/cmd/reva/download.go +++ b/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 { diff --git a/cmd/reva/upload.go b/cmd/reva/upload.go index d5ca0eb17f7..145f4cf0118 100644 --- a/cmd/reva/upload.go +++ b/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 diff --git a/examples/meshdirectory/meshdirectory.toml b/examples/meshdirectory/meshdirectory.toml index c14f9b7fe3f..a56222e568c 100644 --- a/examples/meshdirectory/meshdirectory.toml +++ b/examples/meshdirectory/meshdirectory.toml @@ -13,3 +13,5 @@ driver = "mentix" # #[http.services.meshdirectory.drivers.mentix] #url = "http://localhost:9600/" +#insecure = true +#timeout = 10 diff --git a/internal/http/services/datagateway/datagateway.go b/internal/http/services/datagateway/datagateway.go index d9d42e1abf4..6dea7a3f8e8 100644 --- a/internal/http/services/datagateway/datagateway.go +++ b/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") diff --git a/internal/http/services/dataprovider/dataprovider.go b/internal/http/services/dataprovider/dataprovider.go index ec8b1723b51..c58646751df 100644 --- a/internal/http/services/dataprovider/dataprovider.go +++ b/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() { diff --git a/internal/http/services/dataprovider/put.go b/internal/http/services/dataprovider/put.go index 6471b475d58..5dc93e57c87 100644 --- a/internal/http/services/dataprovider/put.go +++ b/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) diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 82d51d93dd8..db12c31c513 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/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,7 +292,7 @@ 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 } @@ -295,14 +300,18 @@ func descend(ctx context.Context, client gateway.GatewayAPIClient, src *provider 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 diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go index 6afe39d5031..449c12ac7c0 100644 --- a/internal/http/services/owncloud/ocdav/get.go +++ b/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 { diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 00f647c7457..f571b5adf90 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/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() { diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 979b19e81ee..24d8eb576e1 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/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) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 2c322e0f290..b4d6d69cf8d 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/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") diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index 19a7fef8a14..ab6941cc18a 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/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 } diff --git a/pkg/meshdirectory/manager/mentix/mentix.go b/pkg/meshdirectory/manager/mentix/mentix.go index 8da3a5c4294..f1003f4770d 100644 --- a/pkg/meshdirectory/manager/mentix/mentix.go +++ b/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 { diff --git a/pkg/rhttp/client.go b/pkg/rhttp/client.go index 4f41b0087b4..02ff2828ee0 100644 --- a/pkg/rhttp/client.go +++ b/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, }, }, } diff --git a/pkg/user/manager/rest/rest.go b/pkg/user/manager/rest/rest.go index 4a92fc9d2eb..f759e565f9d 100644 --- a/pkg/user/manager/rest/rest.go +++ b/pkg/user/manager/rest/rest.go @@ -151,7 +151,7 @@ func (m *manager) getAPIToken(ctx context.Context) (string, time.Time, error) { "audience": {m.conf.TargetAPI}, } - httpClient := rhttp.GetHTTPClient(ctx) + httpClient := rhttp.GetHTTPClient(rhttp.Context(ctx), rhttp.Timeout(10*time.Second), rhttp.Insecure(true)) httpReq, err := rhttp.NewRequest(ctx, "POST", m.conf.OIDCTokenEndpoint, strings.NewReader(params.Encode())) if err != nil { return "", time.Time{}, err @@ -186,7 +186,7 @@ func (m *manager) sendAPIRequest(ctx context.Context, url string) ([]interface{} return nil, err } - httpClient := rhttp.GetHTTPClient(ctx) + httpClient := rhttp.GetHTTPClient(rhttp.Context(ctx), rhttp.Timeout(10*time.Second), rhttp.Insecure(true)) httpReq, err := rhttp.NewRequest(ctx, "GET", url, nil) if err != nil { return nil, err