From 26d27e2417e1619df14dad1d8bf90702d055b918 Mon Sep 17 00:00:00 2001 From: Joao Marcal Date: Thu, 9 Nov 2023 11:15:16 +0000 Subject: [PATCH] Add HTTP Config to GCS Signed-off-by: Joao Marcal --- CHANGELOG.md | 1 + exthttp/transport.go | 10 +++++++ providers/gcs/gcs.go | 48 ++++++++++++++++++++++++++++----- providers/gcs/gcs_test.go | 57 +++++++++++++++++++++++++++++++++++++++ providers/s3/s3.go | 14 ++-------- 5 files changed, 112 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46b1410e..3d4d3870 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#73](https://github.com/thanos-io/objstore/pull/73) Аdded file path to erros from DownloadFile - [#51](https://github.com/thanos-io/objstore/pull/51) Azure: Support using connection string authentication. - [#76](https://github.com/thanos-io/objstore/pull/76) GCS: Query for object names only in `Iter` to possibly improve performance when listing objects. +- [#86](https://github.com/thanos-io/objstore/pull/86) GCS: Add HTTP Config to GCS ### Changed - [#38](https://github.com/thanos-io/objstore/pull/38) *: Upgrade minio-go version to `v7.0.45`. diff --git a/exthttp/transport.go b/exthttp/transport.go index d9a807fd..92f86a11 100644 --- a/exthttp/transport.go +++ b/exthttp/transport.go @@ -11,6 +11,16 @@ import ( "github.com/prometheus/common/model" ) +var DefaultHTTPConfig = HTTPConfig{ + IdleConnTimeout: model.Duration(90 * time.Second), + ResponseHeaderTimeout: model.Duration(2 * time.Minute), + TLSHandshakeTimeout: model.Duration(10 * time.Second), + ExpectContinueTimeout: model.Duration(1 * time.Second), + MaxIdleConns: 100, + MaxIdleConnsPerHost: 100, + MaxConnsPerHost: 0, +} + // HTTPConfig stores the http.Transport configuration for the cos and s3 minio client. type HTTPConfig struct { IdleConnTimeout model.Duration `yaml:"idle_conn_timeout"` diff --git a/providers/gcs/gcs.go b/providers/gcs/gcs.go index ad305d6e..5dceae67 100644 --- a/providers/gcs/gcs.go +++ b/providers/gcs/gcs.go @@ -8,9 +8,11 @@ import ( "context" "fmt" "io" + "net/http" "runtime" "strings" "testing" + "time" "cloud.google.com/go/storage" "github.com/go-kit/log" @@ -24,15 +26,21 @@ import ( "gopkg.in/yaml.v2" "github.com/thanos-io/objstore" + "github.com/thanos-io/objstore/exthttp" ) // DirDelim is the delimiter used to model a directory structure in an object store bucket. const DirDelim = "/" +var DefaultConfig = Config{ + HTTPConfig: exthttp.DefaultHTTPConfig, +} + // Config stores the configuration for gcs bucket. type Config struct { - Bucket string `yaml:"bucket"` - ServiceAccount string `yaml:"service_account"` + Bucket string `yaml:"bucket"` + ServiceAccount string `yaml:"service_account"` + HTTPConfig exthttp.HTTPConfig `yaml:"http_config"` } // Bucket implements the store.Bucket and shipper.Bucket interfaces against GCS. @@ -44,14 +52,23 @@ type Bucket struct { closer io.Closer } +// parseConfig unmarshals a buffer into a Config with default values. +func parseConfig(conf []byte) (Config, error) { + config := DefaultConfig + if err := yaml.UnmarshalStrict(conf, &config); err != nil { + return Config{}, err + } + + return config, nil +} + // NewBucket returns a new Bucket against the given bucket handle. func NewBucket(ctx context.Context, logger log.Logger, conf []byte, component string) (*Bucket, error) { - var gc Config - if err := yaml.Unmarshal(conf, &gc); err != nil { + config, err := parseConfig(conf) + if err != nil { return nil, err } - - return NewBucketWithConfig(ctx, logger, gc, component) + return NewBucketWithConfig(ctx, logger, config, component) } // NewBucketWithConfig returns a new Bucket with gcs Config struct. @@ -71,6 +88,25 @@ func NewBucketWithConfig(ctx context.Context, logger log.Logger, gc Config, comp opts = append(opts, option.WithCredentials(credentials)) } + // Check if a roundtripper has been set in the config + // otherwise build the default transport. + var rt http.RoundTripper + if gc.HTTPConfig.Transport != nil { + rt = gc.HTTPConfig.Transport + } else { + var err error + rt, err = exthttp.DefaultTransport(gc.HTTPConfig) + if err != nil { + return nil, err + } + } + + httpCli := &http.Client{ + Transport: rt, + Timeout: time.Duration(gc.HTTPConfig.IdleConnTimeout), + } + opts = append(opts, option.WithHTTPClient(httpCli)) + opts = append(opts, option.WithUserAgent(fmt.Sprintf("thanos-%s/%s (%s)", component, version.Version, runtime.Version())), ) diff --git a/providers/gcs/gcs_test.go b/providers/gcs/gcs_test.go index 283de6b6..95e0b005 100644 --- a/providers/gcs/gcs_test.go +++ b/providers/gcs/gcs_test.go @@ -10,9 +10,11 @@ import ( "net/http/httptest" "os" "testing" + "time" "github.com/efficientgo/core/testutil" "github.com/go-kit/log" + "github.com/prometheus/common/model" ) func TestBucket_Get_ShouldReturnErrorIfServerTruncateResponse(t *testing.T) { @@ -44,3 +46,58 @@ func TestBucket_Get_ShouldReturnErrorIfServerTruncateResponse(t *testing.T) { testutil.NotOk(t, err) testutil.Equals(t, "storage: partial request not satisfied", err.Error()) } + +func TestParseConfig_HTTPConfig(t *testing.T) { + for _, tc := range []struct { + name string + input string + assertions func(cfg Config) + }{ + { + name: "DefaultHTTPConfig", + input: `bucket: abcd`, + assertions: func(cfg Config) { + testutil.Equals(t, cfg.HTTPConfig.IdleConnTimeout, model.Duration(90*time.Second)) + testutil.Equals(t, cfg.HTTPConfig.ResponseHeaderTimeout, model.Duration(2*time.Minute)) + testutil.Equals(t, cfg.HTTPConfig.InsecureSkipVerify, false) + }, + }, + { + name: "CustomHTTPConfig", + input: `bucket: abcd +http_config: + insecure_skip_verify: true + idle_conn_timeout: 50s + response_header_timeout: 1m`, + assertions: func(cfg Config) { + testutil.Equals(t, cfg.HTTPConfig.IdleConnTimeout, model.Duration(50*time.Second)) + testutil.Equals(t, cfg.HTTPConfig.ResponseHeaderTimeout, model.Duration(1*time.Minute)) + testutil.Equals(t, cfg.HTTPConfig.InsecureSkipVerify, true) + }, + }, + { + name: "CustomHTTPConfigWithTLS", + input: `bucket: abcd +http_config: + tls_config: + ca_file: /certs/ca.crt + cert_file: /certs/cert.crt + key_file: /certs/key.key + server_name: server + insecure_skip_verify: false`, + assertions: func(cfg Config) { + testutil.Equals(t, "/certs/ca.crt", cfg.HTTPConfig.TLSConfig.CAFile) + testutil.Equals(t, "/certs/cert.crt", cfg.HTTPConfig.TLSConfig.CertFile) + testutil.Equals(t, "/certs/key.key", cfg.HTTPConfig.TLSConfig.KeyFile) + testutil.Equals(t, "server", cfg.HTTPConfig.TLSConfig.ServerName) + testutil.Equals(t, false, cfg.HTTPConfig.TLSConfig.InsecureSkipVerify) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg, err := parseConfig([]byte(tc.input)) + testutil.Ok(t, err) + tc.assertions(cfg) + }) + } +} diff --git a/providers/s3/s3.go b/providers/s3/s3.go index 83e3a2de..38ddbde5 100644 --- a/providers/s3/s3.go +++ b/providers/s3/s3.go @@ -14,7 +14,6 @@ import ( "strconv" "strings" "testing" - "time" "github.com/efficientgo/core/logerrcapture" "github.com/go-kit/log" @@ -23,7 +22,6 @@ import ( "github.com/minio/minio-go/v7/pkg/credentials" "github.com/minio/minio-go/v7/pkg/encrypt" "github.com/pkg/errors" - "github.com/prometheus/common/model" "github.com/prometheus/common/version" "gopkg.in/yaml.v2" @@ -101,16 +99,8 @@ const ( ) var DefaultConfig = Config{ - PutUserMetadata: map[string]string{}, - HTTPConfig: exthttp.HTTPConfig{ - IdleConnTimeout: model.Duration(90 * time.Second), - ResponseHeaderTimeout: model.Duration(2 * time.Minute), - TLSHandshakeTimeout: model.Duration(10 * time.Second), - ExpectContinueTimeout: model.Duration(1 * time.Second), - MaxIdleConns: 100, - MaxIdleConnsPerHost: 100, - MaxConnsPerHost: 0, - }, + PutUserMetadata: map[string]string{}, + HTTPConfig: exthttp.DefaultHTTPConfig, PartSize: 1024 * 1024 * 64, // 64MB. BucketLookupType: AutoLookup, }