From 677d7cc2f75d36541d24c12b74df6b516001af67 Mon Sep 17 00:00:00 2001 From: Doug Simmons Date: Mon, 20 Jul 2020 16:37:21 -0700 Subject: [PATCH] Add configurable timeout for config, convert, and validate plugins Also run 'go fmt' on changed files --- cmd/drone-server/config/config.go | 25 ++++++++++++++----------- cmd/drone-server/inject_plugin.go | 3 +++ plugin/config/global.go | 12 +++++++----- plugin/config/global_oss.go | 3 ++- plugin/config/global_test.go | 12 ++++++++---- plugin/converter/remote.go | 10 ++++++---- plugin/converter/remote_oss.go | 4 +++- plugin/converter/remote_test.go | 4 +++- plugin/validator/remote.go | 10 ++++++---- plugin/validator/remote_oss.go | 4 +++- 10 files changed, 55 insertions(+), 32 deletions(-) diff --git a/cmd/drone-server/config/config.go b/cmd/drone-server/config/config.go index 2d9bd4448..d7b8a24ef 100644 --- a/cmd/drone-server/config/config.go +++ b/cmd/drone-server/config/config.go @@ -98,7 +98,7 @@ type ( } Cleanup struct { - Disabled bool `envconfig:"DRONE_CLEANUP_DISABLED"` + Disabled bool `envconfig:"DRONE_CLEANUP_DISABLED"` Interval time.Duration `envconfig:"DRONE_CLEANUP_INTERVAL" default:"24h"` Running time.Duration `envconfig:"DRONE_CLEANUP_DEADLINE_RUNNING" default:"24h"` Pending time.Duration `envconfig:"DRONE_CLEANUP_DEADLINE_PENDING" default:"24h"` @@ -308,24 +308,27 @@ type ( // Yaml provides the yaml webhook configuration. Yaml struct { - Endpoint string `envconfig:"DRONE_YAML_ENDPOINT"` - Secret string `envconfig:"DRONE_YAML_SECRET"` - SkipVerify bool `envconfig:"DRONE_YAML_SKIP_VERIFY"` + Endpoint string `envconfig:"DRONE_YAML_ENDPOINT"` + Secret string `envconfig:"DRONE_YAML_SECRET"` + SkipVerify bool `envconfig:"DRONE_YAML_SKIP_VERIFY"` + Timeout time.Duration `envconfig:"DRONE_YAML_TIMEOUT" default:"1m"` } // Convert provides the converter webhook configuration. Convert struct { - Extension string `envconfig:"DRONE_CONVERT_PLUGIN_EXTENSION"` - Endpoint string `envconfig:"DRONE_CONVERT_PLUGIN_ENDPOINT"` - Secret string `envconfig:"DRONE_CONVERT_PLUGIN_SECRET"` - SkipVerify bool `envconfig:"DRONE_CONVERT_PLUGIN_SKIP_VERIFY"` + Extension string `envconfig:"DRONE_CONVERT_PLUGIN_EXTENSION"` + Endpoint string `envconfig:"DRONE_CONVERT_PLUGIN_ENDPOINT"` + Secret string `envconfig:"DRONE_CONVERT_PLUGIN_SECRET"` + SkipVerify bool `envconfig:"DRONE_CONVERT_PLUGIN_SKIP_VERIFY"` + Timeout time.Duration `envconfig:"DRONE_CONVERT_TIMEOUT" default:"1m"` } // Validate provides the validation webhook configuration. Validate struct { - Endpoint string `envconfig:"DRONE_VALIDATE_PLUGIN_ENDPOINT"` - Secret string `envconfig:"DRONE_VALIDATE_PLUGIN_SECRET"` - SkipVerify bool `envconfig:"DRONE_VALIDATE_PLUGIN_SKIP_VERIFY"` + Endpoint string `envconfig:"DRONE_VALIDATE_PLUGIN_ENDPOINT"` + Secret string `envconfig:"DRONE_VALIDATE_PLUGIN_SECRET"` + SkipVerify bool `envconfig:"DRONE_VALIDATE_PLUGIN_SKIP_VERIFY"` + Timeout time.Duration `envconfig:"DRONE_VALIDATE_TIMEOUT" default:"1m"` } // diff --git a/cmd/drone-server/inject_plugin.go b/cmd/drone-server/inject_plugin.go index 8c2f2a04d..0fdd5a84f 100644 --- a/cmd/drone-server/inject_plugin.go +++ b/cmd/drone-server/inject_plugin.go @@ -65,6 +65,7 @@ func provideConfigPlugin(client *scm.Client, contents core.FileService, conf spe conf.Yaml.Endpoint, conf.Yaml.Secret, conf.Yaml.SkipVerify, + conf.Yaml.Timeout, ), config.Repository(contents), ) @@ -85,6 +86,7 @@ func provideConvertPlugin(client *scm.Client, conf spec.Config) core.ConvertServ conf.Convert.Secret, conf.Convert.Extension, conf.Convert.SkipVerify, + conf.Convert.Timeout, ), ) } @@ -129,6 +131,7 @@ func provideValidatePlugin(conf spec.Config) core.ValidateService { conf.Validate.Endpoint, conf.Validate.Secret, conf.Validate.SkipVerify, + conf.Validate.Timeout, ), ) } diff --git a/plugin/config/global.go b/plugin/config/global.go index e95dba1e9..7364603ad 100644 --- a/plugin/config/global.go +++ b/plugin/config/global.go @@ -12,12 +12,13 @@ import ( "github.com/drone/drone-go/drone" "github.com/drone/drone-go/plugin/config" + "github.com/drone/drone/core" ) // Global returns a configuration service that fetches the yaml // configuration from a remote endpoint. -func Global(endpoint, signer string, skipVerify bool) core.ConfigService { +func Global(endpoint, signer string, skipVerify bool, timeout time.Duration) core.ConfigService { if endpoint == "" { return new(global) } @@ -27,23 +28,24 @@ func Global(endpoint, signer string, skipVerify bool) core.ConfigService { signer, skipVerify, ), + timeout: timeout, } } type global struct { client config.Plugin + timeout time.Duration } func (g *global) Find(ctx context.Context, in *core.ConfigArgs) (*core.Config, error) { if g.client == nil { return nil, nil } - // include a timeout to prevent an API call from // hanging the build process indefinitely. The - // external service must return a request within - // one minute. - ctx, cancel := context.WithTimeout(ctx, time.Minute) + // external service must return a response within + // the configured timeout (default 1m). + ctx, cancel := context.WithTimeout(ctx, g.timeout) defer cancel() req := &config.Request{ diff --git a/plugin/config/global_oss.go b/plugin/config/global_oss.go index 8d4243fce..b75a2b3a9 100644 --- a/plugin/config/global_oss.go +++ b/plugin/config/global_oss.go @@ -18,12 +18,13 @@ package config import ( "context" + "time" "github.com/drone/drone/core" ) // Global returns a no-op configuration service. -func Global(string, string, bool) core.ConfigService { +func Global(string, string, bool, time.Duration) core.ConfigService { return new(noop) } diff --git a/plugin/config/global_test.go b/plugin/config/global_test.go index d17b1c8da..3d57ce759 100644 --- a/plugin/config/global_test.go +++ b/plugin/config/global_test.go @@ -8,6 +8,7 @@ package config import ( "testing" + "time" "github.com/drone/drone/core" "github.com/h2non/gock" @@ -31,7 +32,8 @@ func TestGlobal(t *testing.T) { Build: &core.Build{After: "6d144de7"}, } - service := Global("https://company.com/config", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", false) + service := Global("https://company.com/config", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", + false, time.Minute) result, err := service.Find(noContext, args) if err != nil { t.Error(err) @@ -65,7 +67,8 @@ func TestGlobalErr(t *testing.T) { Build: &core.Build{After: "6d144de7"}, } - service := Global("https://company.com/config", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", false) + service := Global("https://company.com/config", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", + false, time.Minute) _, err := service.Find(noContext, args) if err == nil { t.Errorf("Expect http.Reponse error") @@ -95,7 +98,8 @@ func TestGlobalEmpty(t *testing.T) { Build: &core.Build{After: "6d144de7"}, } - service := Global("https://company.com/config", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", false) + service := Global("https://company.com/config", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", + false, time.Minute) result, err := service.Find(noContext, args) if err != nil { t.Error(err) @@ -112,7 +116,7 @@ func TestGlobalEmpty(t *testing.T) { } func TestGlobalDisabled(t *testing.T) { - res, err := Global("", "", false).Find(noContext, nil) + res, err := Global("", "", false, time.Minute).Find(noContext, nil) if err != nil { t.Error(err) } diff --git a/plugin/converter/remote.go b/plugin/converter/remote.go index 1fe866304..482a80f91 100644 --- a/plugin/converter/remote.go +++ b/plugin/converter/remote.go @@ -18,7 +18,7 @@ import ( // Remote returns a conversion service that converts the // configuration file using a remote http service. -func Remote(endpoint, signer, extension string, skipVerify bool) core.ConvertService { +func Remote(endpoint, signer, extension string, skipVerify bool, timeout time.Duration) core.ConvertService { if endpoint == "" { return new(remote) } @@ -29,12 +29,14 @@ func Remote(endpoint, signer, extension string, skipVerify bool) core.ConvertSer signer, skipVerify, ), + timeout: timeout, } } type remote struct { client converter.Plugin extension string + timeout time.Duration } func (g *remote) Convert(ctx context.Context, in *core.ConvertArgs) (*core.Config, error) { @@ -48,9 +50,9 @@ func (g *remote) Convert(ctx context.Context, in *core.ConvertArgs) (*core.Confi } // include a timeout to prevent an API call from // hanging the build process indefinitely. The - // external service must return a request within - // one minute. - ctx, cancel := context.WithTimeout(ctx, time.Minute) + // external service must return a response within + // the configured timeout (default 1m). + ctx, cancel := context.WithTimeout(ctx, g.timeout) defer cancel() req := &converter.Request{ diff --git a/plugin/converter/remote_oss.go b/plugin/converter/remote_oss.go index bbd25290d..6a9fc9f2f 100644 --- a/plugin/converter/remote_oss.go +++ b/plugin/converter/remote_oss.go @@ -17,11 +17,13 @@ package converter import ( + "time" + "github.com/drone/drone/core" ) // Remote returns a conversion service that converts the // configuration file using a remote http service. -func Remote(endpoint, signer, extension string, skipVerify bool) core.ConvertService { +func Remote(endpoint, signer, extension string, skipVerify bool, timeout time.Duration) core.ConvertService { return new(noop) } diff --git a/plugin/converter/remote_test.go b/plugin/converter/remote_test.go index 3f3097520..b5bf4c99a 100644 --- a/plugin/converter/remote_test.go +++ b/plugin/converter/remote_test.go @@ -9,6 +9,7 @@ package converter import ( "context" "testing" + "time" "github.com/drone/drone/core" "github.com/h2non/gock" @@ -35,7 +36,8 @@ func TestConvert(t *testing.T) { }, } - service := Remote("https://company.com/convert", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", "", false) + service := Remote("https://company.com/convert", "GMEuUHQfmrMRsseWxi9YlIeBtn9lm6im", "", + false, time.Minute) result, err := service.Convert(context.Background(), args) if err != nil { t.Error(err) diff --git a/plugin/validator/remote.go b/plugin/validator/remote.go index 78370ae22..1bdfb4391 100644 --- a/plugin/validator/remote.go +++ b/plugin/validator/remote.go @@ -17,11 +17,12 @@ import ( // Remote returns a conversion service that converts the // configuration file using a remote http service. -func Remote(endpoint, signer string, skipVerify bool) core.ValidateService { +func Remote(endpoint, signer string, skipVerify bool, timeout time.Duration) core.ValidateService { return &remote{ endpoint: endpoint, secret: signer, skipVerify: skipVerify, + timeout: timeout, } } @@ -29,6 +30,7 @@ type remote struct { endpoint string secret string skipVerify bool + timeout time.Duration } func (g *remote) Validate(ctx context.Context, in *core.ValidateArgs) error { @@ -37,9 +39,9 @@ func (g *remote) Validate(ctx context.Context, in *core.ValidateArgs) error { } // include a timeout to prevent an API call from // hanging the build process indefinitely. The - // external service must return a request within - // one minute. - ctx, cancel := context.WithTimeout(ctx, time.Minute) + // external service must return a response within + // the configured timeout (default 1m). + ctx, cancel := context.WithTimeout(ctx, g.timeout) defer cancel() req := &validator.Request{ diff --git a/plugin/validator/remote_oss.go b/plugin/validator/remote_oss.go index 23165c410..c6a9d4c16 100644 --- a/plugin/validator/remote_oss.go +++ b/plugin/validator/remote_oss.go @@ -17,11 +17,13 @@ package validator import ( + "time" + "github.com/drone/drone/core" ) // Remote returns a conversion service that converts the // configuration file using a remote http service. -func Remote(endpoint, signer string, skipVerify bool) core.ValidateService { +func Remote(endpoint, signer string, skipVerify bool, timeout time.Duration) core.ValidateService { return new(noop) }