diff --git a/cmd/proxy/actions/app.go b/cmd/proxy/actions/app.go index 26ac262c..5c5de769 100644 --- a/cmd/proxy/actions/app.go +++ b/cmd/proxy/actions/app.go @@ -33,7 +33,7 @@ func App(conf *config.Config) (*buffalo.App, error) { // ENV is used to help switch settings based on where the // application is being run. Default is "development". ENV := conf.GoEnv - store, err := GetStorage(conf.StorageType, conf.Storage) + store, err := GetStorage(conf.StorageType, conf.Storage, conf.TimeoutDuration()) if err != nil { err = fmt.Errorf("error getting storage configuration (%s)", err) return nil, err diff --git a/cmd/proxy/actions/storage.go b/cmd/proxy/actions/storage.go index 889f5947..320d1f66 100644 --- a/cmd/proxy/actions/storage.go +++ b/cmd/proxy/actions/storage.go @@ -3,6 +3,7 @@ package actions import ( "context" "fmt" + "time" "github.com/gomods/athens/pkg/config" "github.com/gomods/athens/pkg/errors" @@ -17,7 +18,7 @@ import ( ) // GetStorage returns storage backend based on env configuration -func GetStorage(storageType string, storageConfig *config.StorageConfig) (storage.Backend, error) { +func GetStorage(storageType string, storageConfig *config.StorageConfig, timeout time.Duration) (storage.Backend, error) { const op errors.Op = "actions.GetStorage" switch storageType { case "memory": @@ -26,7 +27,7 @@ func GetStorage(storageType string, storageConfig *config.StorageConfig) (storag if storageConfig.Mongo == nil { return nil, errors.E(op, "Invalid Mongo Storage Configuration") } - return mongo.NewStorage(storageConfig.Mongo) + return mongo.NewStorage(storageConfig.Mongo, timeout) case "disk": if storageConfig.Disk == nil { return nil, errors.E(op, "Invalid Disk Storage Configuration") @@ -42,17 +43,17 @@ func GetStorage(storageType string, storageConfig *config.StorageConfig) (storag if storageConfig.Minio == nil { return nil, errors.E(op, "Invalid Minio Storage Configuration") } - return minio.NewStorage(storageConfig.Minio) + return minio.NewStorage(storageConfig.Minio, timeout) case "gcp": if storageConfig.GCP == nil { return nil, errors.E(op, "Invalid GCP Storage Configuration") } - return gcp.New(context.Background(), storageConfig.GCP) + return gcp.New(context.Background(), storageConfig.GCP, timeout) case "s3": if storageConfig.S3 == nil { return nil, errors.E(op, "Invalid S3 Storage Configuration") } - return s3.New(storageConfig.S3) + return s3.New(storageConfig.S3, timeout) default: return nil, fmt.Errorf("storage type %s is unknown", storageType) } diff --git a/config.dev.toml b/config.dev.toml index ecaeab3c..434f3fae 100644 --- a/config.dev.toml +++ b/config.dev.toml @@ -136,10 +136,6 @@ TraceExporter = "" # Env override: CDN_ENDPOINT Endpoint = "cdn.example.com" - # Timeout for networks calls made to the CDN in seconds - # Defaults to Global Timeout - Timeout = 300 - [Storage.Disk] # RootPath is the Athens Disk Root folder # Env override: ATHENS_DISK_STORAGE_ROOT @@ -154,10 +150,6 @@ TraceExporter = "" # Env override: ATHENS_STORAGE_GCP_BUCKET Bucket = "MY_GCP_BUCKET" - # Timeout for networks calls made to GCP in seconds - # Defaults to Global Timeout - Timeout = 300 - [Storage.Minio] # Endpoint for Minio storage # Env override: ATHENS_MINIO_ENDPOINT @@ -171,10 +163,6 @@ TraceExporter = "" # Env override: ATHENS_MINIO_SECRET_ACCESS_KEY Secret = "minio123" - # Timeout for networks calls made to Minio in seconds - # Defaults to Global Timeout - Timeout = 300 - # Enable SSL for Minio connections # Defaults to true # Env override: ATHENS_MINIO_USE_SSL @@ -198,11 +186,6 @@ TraceExporter = "" # Env override: ATHENS_MONGO_CERT_PATH CertPath = "" - # Timeout for networks calls made to Mongo in seconds - # Defaults to Global Timeout - # Env override: MONGO_CONN_TIMEOUT_SEC - Timeout = 300 - # Allows for insecure SSL / http connections to mongo storage # Should be used for testing or development only # Env override: ATHENS_MONGO_INSECURE @@ -225,10 +208,6 @@ TraceExporter = "" # Env override: AWS_SESSION_TOKEN Token = "" - # Timeout for networks calls made to S3 in seconds - # Defaults to Global Timeout - Timeout = 300 - # S3 Bucket to use for storage # Defaults to gomods # Env override: ATHENS_S3_BUCKET_NAME diff --git a/pkg/config/config.go b/pkg/config/config.go index e4eec434..b8f0e9e5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -71,9 +71,6 @@ func ParseConfigFile(configFile string) (*Config, error) { return nil, err } - // If not defined, set storage timeouts to global timeout - setStorageTimeouts(config.Storage, config.Timeout) - // validate all required fields have been populated if err := validateConfig(config); err != nil { return nil, err diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4945253e..6a2dbb0b 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -98,7 +98,6 @@ func TestEnvOverrides(t *testing.T) { } func TestStorageEnvOverrides(t *testing.T) { - globalTimeout := 300 expStorage := &StorageConfig{ Disk: &DiskConfig{ RootPath: "/my/root/path", @@ -106,9 +105,6 @@ func TestStorageEnvOverrides(t *testing.T) { GCP: &GCPConfig{ ProjectID: "gcpproject", Bucket: "gcpbucket", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, Minio: &MinioConfig{ Endpoint: "minioEndpoint", @@ -117,16 +113,10 @@ func TestStorageEnvOverrides(t *testing.T) { EnableSSL: false, Bucket: "minioBucket", Region: "us-west-1", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, Mongo: &MongoConfig{ URL: "mongoURL", CertPath: "/test/path", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, S3: &S3Config{ Region: "s3Region", @@ -134,9 +124,6 @@ func TestStorageEnvOverrides(t *testing.T) { Secret: "s3Secret", Token: "s3Token", Bucket: "s3Bucket", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, } envVars := getEnvMap(&Config{Storage: expStorage}) @@ -151,7 +138,6 @@ func TestStorageEnvOverrides(t *testing.T) { if err != nil { t.Fatalf("Env override failed: %v", err) } - setStorageTimeouts(conf.Storage, globalTimeout) compareStorageConfigs(conf.Storage, expStorage, t) restoreEnv(envVarBackup) } @@ -181,8 +167,6 @@ func TestParseExampleConfig(t *testing.T) { os.Unsetenv(k) } - globalTimeout := 300 - expStorage := &StorageConfig{ Disk: &DiskConfig{ RootPath: "/path/on/disk", @@ -190,9 +174,6 @@ func TestParseExampleConfig(t *testing.T) { GCP: &GCPConfig{ ProjectID: "MY_GCP_PROJECT_ID", Bucket: "MY_GCP_BUCKET", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, Minio: &MinioConfig{ Endpoint: "127.0.0.1:9001", @@ -200,16 +181,10 @@ func TestParseExampleConfig(t *testing.T) { Secret: "minio123", EnableSSL: false, Bucket: "gomods", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, Mongo: &MongoConfig{ - URL: "mongodb://127.0.0.1:27017", - CertPath: "", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, + URL: "mongodb://127.0.0.1:27017", + CertPath: "", InsecureConn: false, }, S3: &S3Config{ @@ -218,9 +193,6 @@ func TestParseExampleConfig(t *testing.T) { Secret: "MY_AWS_SECRET_ACCESS_KEY", Token: "", Bucket: "MY_S3_BUCKET_NAME", - TimeoutConf: TimeoutConf{ - Timeout: globalTimeout, - }, }, } diff --git a/pkg/config/gcp.go b/pkg/config/gcp.go index 45f90117..7b1bd47d 100644 --- a/pkg/config/gcp.go +++ b/pkg/config/gcp.go @@ -2,7 +2,6 @@ package config // GCPConfig specifies the properties required to use GCP as the storage backend type GCPConfig struct { - TimeoutConf ProjectID string `envconfig:"GOOGLE_CLOUD_PROJECT"` Bucket string `validate:"required" envconfig:"ATHENS_STORAGE_GCP_BUCKET"` } diff --git a/pkg/config/minio.go b/pkg/config/minio.go index b7251f6c..fc5885f4 100644 --- a/pkg/config/minio.go +++ b/pkg/config/minio.go @@ -3,7 +3,6 @@ package config // MinioConfig specifies the properties required to use Minio or DigitalOcean Spaces // as the storage backend type MinioConfig struct { - TimeoutConf Endpoint string `validate:"required" envconfig:"ATHENS_MINIO_ENDPOINT"` Key string `validate:"required" envconfig:"ATHENS_MINIO_ACCESS_KEY_ID"` Secret string `validate:"required" envconfig:"ATHENS_MINIO_SECRET_ACCESS_KEY"` diff --git a/pkg/config/mongo.go b/pkg/config/mongo.go index bee48251..ea7a4dce 100644 --- a/pkg/config/mongo.go +++ b/pkg/config/mongo.go @@ -2,7 +2,6 @@ package config // MongoConfig specifies the properties required to use MongoDB as the storage backend type MongoConfig struct { - TimeoutConf URL string `validate:"required" envconfig:"ATHENS_MONGO_STORAGE_URL"` CertPath string `envconfig:"ATHENS_MONGO_CERT_PATH"` InsecureConn bool `envconfig:"ATHENS_MONGO_INSECURE"` diff --git a/pkg/config/s3.go b/pkg/config/s3.go index 029983d2..f5268522 100644 --- a/pkg/config/s3.go +++ b/pkg/config/s3.go @@ -2,7 +2,6 @@ package config // S3Config specifies the properties required to use S3 as the storage backend type S3Config struct { - TimeoutConf Region string `validate:"required" envconfig:"AWS_REGION"` Key string `validate:"required" envconfig:"AWS_ACCESS_KEY_ID"` Secret string `validate:"required" envconfig:"AWS_SECRET_ACCESS_KEY"` diff --git a/pkg/config/storage.go b/pkg/config/storage.go index 9169bbb7..986bf25c 100644 --- a/pkg/config/storage.go +++ b/pkg/config/storage.go @@ -9,24 +9,3 @@ type StorageConfig struct { S3 *S3Config Azure *AzureConfig } - -func setStorageTimeouts(s *StorageConfig, defaultTimeout int) { - if s == nil { - return - } - if s.GCP != nil && s.GCP.Timeout == 0 { - s.GCP.Timeout = defaultTimeout - } - if s.Minio != nil && s.Minio.Timeout == 0 { - s.Minio.Timeout = defaultTimeout - } - if s.Mongo != nil && s.Mongo.Timeout == 0 { - s.Mongo.Timeout = defaultTimeout - } - if s.S3 != nil && s.S3.Timeout == 0 { - s.S3.Timeout = defaultTimeout - } - if s.Azure != nil && s.Azure.Timeout == 0 { - s.Azure.Timeout = defaultTimeout - } -} diff --git a/pkg/config/timeout.go b/pkg/config/timeout.go index b039c8b8..8e86b84f 100644 --- a/pkg/config/timeout.go +++ b/pkg/config/timeout.go @@ -9,5 +9,10 @@ type TimeoutConf struct { // TimeoutDuration returns the timeout as time.duration func (t *TimeoutConf) TimeoutDuration() time.Duration { - return time.Second * time.Duration(t.Timeout) + return GetTimeoutDuration(t.Timeout) +} + +// GetTimeoutDuration returns the timeout as time.duration +func GetTimeoutDuration(timeout int) time.Duration { + return time.Second * time.Duration(timeout) } diff --git a/pkg/storage/gcp/gcp.go b/pkg/storage/gcp/gcp.go index 8cc2d7a1..1259e4ec 100644 --- a/pkg/storage/gcp/gcp.go +++ b/pkg/storage/gcp/gcp.go @@ -30,7 +30,7 @@ type Storage struct { // to the path of your service account file. If you're running on GCP (e.g. AppEngine), // credentials will be automatically provided. // See https://cloud.google.com/docs/authentication/getting-started. -func New(ctx context.Context, gcpConf *config.GCPConfig) (*Storage, error) { +func New(ctx context.Context, gcpConf *config.GCPConfig, timeout time.Duration) (*Storage, error) { const op errors.Op = "gcp.New" storage, err := storage.NewClient(ctx) if err != nil { @@ -51,7 +51,7 @@ func New(ctx context.Context, gcpConf *config.GCPConfig) (*Storage, error) { bucket: &bkt, baseURI: u, closeStorage: storage.Close, - timeout: gcpConf.TimeoutDuration(), + timeout: timeout, }, nil } diff --git a/pkg/storage/minio/minio.go b/pkg/storage/minio/minio.go index e60974ef..01a87be1 100644 --- a/pkg/storage/minio/minio.go +++ b/pkg/storage/minio/minio.go @@ -2,6 +2,7 @@ package minio import ( "fmt" + "time" "github.com/gomods/athens/pkg/config" "github.com/gomods/athens/pkg/errors" @@ -20,7 +21,7 @@ func (s *storageImpl) versionLocation(module, version string) string { // NewStorage returns a connected Minio or DigitalOcean Spaces storage // that implements storage.Backend -func NewStorage(conf *config.MinioConfig) (storage.Backend, error) { +func NewStorage(conf *config.MinioConfig, timeout time.Duration) (storage.Backend, error) { const op errors.Op = "minio.NewStorage" endpoint := conf.Endpoint accessKeyID := conf.Key diff --git a/pkg/storage/minio/minio_test.go b/pkg/storage/minio/minio_test.go index 4a369ea1..6fd14f70 100644 --- a/pkg/storage/minio/minio_test.go +++ b/pkg/storage/minio/minio_test.go @@ -44,7 +44,7 @@ func getStorage(t testing.TB) *storageImpl { Key: "minio", Secret: "minio123", Bucket: "gomods", - }) + }, config.GetTimeoutDuration(300)) if err != nil { t.Fatal(err) } diff --git a/pkg/storage/mongo/mongo.go b/pkg/storage/mongo/mongo.go index ff379de5..f0f5493e 100644 --- a/pkg/storage/mongo/mongo.go +++ b/pkg/storage/mongo/mongo.go @@ -27,12 +27,12 @@ type ModuleStore struct { // NewStorage returns a connected Mongo backed storage // that satisfies the Backend interface. -func NewStorage(conf *config.MongoConfig) (*ModuleStore, error) { +func NewStorage(conf *config.MongoConfig, timeout time.Duration) (*ModuleStore, error) { const op errors.Op = "mongo.NewStorage" if conf == nil { return nil, errors.E(op, "No Mongo Configuration provided") } - ms := &ModuleStore{url: conf.URL, certPath: conf.CertPath, timeout: conf.TimeoutDuration()} + ms := &ModuleStore{url: conf.URL, certPath: conf.CertPath, timeout: timeout} err := ms.connect() if err != nil { diff --git a/pkg/storage/mongo/mongo_test.go b/pkg/storage/mongo/mongo_test.go index 7870f2cb..5e8bf2bc 100644 --- a/pkg/storage/mongo/mongo_test.go +++ b/pkg/storage/mongo/mongo_test.go @@ -31,7 +31,7 @@ func getStorage(tb testing.TB) *ModuleStore { tb.SkipNow() } - backend, err := NewStorage(&config.MongoConfig{URL: url}) + backend, err := NewStorage(&config.MongoConfig{URL: url}, config.GetTimeoutDuration(300)) require.NoError(tb, err) return backend diff --git a/pkg/storage/s3/deleter.go b/pkg/storage/s3/deleter.go index 0fcdf1e9..c2589e9f 100644 --- a/pkg/storage/s3/deleter.go +++ b/pkg/storage/s3/deleter.go @@ -25,7 +25,7 @@ func (s *Storage) Delete(ctx context.Context, module, version string) error { return errors.E(op, errors.M(module), errors.V(version), errors.KindNotFound) } - return modupl.Delete(ctx, module, version, s.remove, s.s3Conf.TimeoutDuration()) + return modupl.Delete(ctx, module, version, s.remove, s.timeout) } func (s *Storage) remove(ctx context.Context, path string) error { diff --git a/pkg/storage/s3/s3.go b/pkg/storage/s3/s3.go index d513d0ec..4121b059 100644 --- a/pkg/storage/s3/s3.go +++ b/pkg/storage/s3/s3.go @@ -3,6 +3,7 @@ package s3 import ( "fmt" "net/url" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/credentials" @@ -27,11 +28,11 @@ type Storage struct { baseURI *url.URL uploader s3manageriface.UploaderAPI s3API s3iface.S3API - s3Conf *config.S3Config + timeout time.Duration } // New creates a new AWS S3 CDN saver -func New(s3Conf *config.S3Config, options ...func(*aws.Config)) (*Storage, error) { +func New(s3Conf *config.S3Config, timeout time.Duration, options ...func(*aws.Config)) (*Storage, error) { const op errors.Op = "s3.New" u, err := url.Parse(fmt.Sprintf("https://%s.s3.amazonaws.com", s3Conf.Bucket)) if err != nil { @@ -59,6 +60,6 @@ func New(s3Conf *config.S3Config, options ...func(*aws.Config)) (*Storage, error uploader: uploader, s3API: uploader.S3, baseURI: u, - s3Conf: s3Conf, + timeout: timeout, }, nil } diff --git a/pkg/storage/s3/s3_test.go b/pkg/storage/s3/s3_test.go index 4414baa4..47e024dd 100644 --- a/pkg/storage/s3/s3_test.go +++ b/pkg/storage/s3/s3_test.go @@ -22,7 +22,7 @@ func BenchmarkBackend(b *testing.B) { } func (s *Storage) clear() error { - ctx, cancel := context.WithTimeout(context.Background(), s.s3Conf.TimeoutDuration()) + ctx, cancel := context.WithTimeout(context.Background(), s.timeout) defer cancel() objects, err := s.s3API.ListObjectsWithContext(ctx, &s3.ListObjectsInput{Bucket: aws.String(s.bucket)}) @@ -45,7 +45,7 @@ func (s *Storage) clear() error { } func (s *Storage) createBucket() error { - ctx, cancel := context.WithTimeout(context.Background(), s.s3Conf.TimeoutDuration()) + ctx, cancel := context.WithTimeout(context.Background(), s.timeout) defer cancel() if _, err := s.s3API.CreateBucketWithContext(ctx, &s3.CreateBucketInput{Bucket: aws.String(s.bucket)}); err != nil { @@ -79,10 +79,8 @@ func getStorage(t testing.TB) *Storage { Secret: "minio123", Bucket: "gomodsaws", Region: "us-west-1", - TimeoutConf: config.TimeoutConf{ - Timeout: 300, - }, }, + config.GetTimeoutDuration(300), options, ) diff --git a/pkg/storage/s3/saver.go b/pkg/storage/s3/saver.go index 05ce2f93..990c15ca 100644 --- a/pkg/storage/s3/saver.go +++ b/pkg/storage/s3/saver.go @@ -17,7 +17,7 @@ func (s *Storage) Save(ctx context.Context, module, version string, mod []byte, const op errors.Op = "s3.Save" ctx, span := observ.StartSpan(ctx, op.String()) defer span.End() - err := moduploader.Upload(ctx, module, version, bytes.NewReader(info), bytes.NewReader(mod), zip, s.upload, s.s3Conf.TimeoutDuration()) + err := moduploader.Upload(ctx, module, version, bytes.NewReader(info), bytes.NewReader(mod), zip, s.upload, s.timeout) // TODO: take out lease on the /list file and add the version to it // // Do that only after module source+metadata is uploaded