web hooks: improve resilience by adding a configurable retry

the retryable http client is used for hooks that notify events

common/actions.go

@@ -34,7 +34,7 @@ type ProtocolActions struct {
 	Hook string `json:"hook" mapstructure:"hook"`
 }
 
-var actionHandler ActionHandler = defaultActionHandler{}
+var actionHandler ActionHandler = &defaultActionHandler{}
 
 // InitializeActionHandler lets the user choose an action handler implementation.
 //
@@ -116,7 +116,7 @@ func newActionNotification(
 
 type defaultActionHandler struct{}
 
-func (h defaultActionHandler) Handle(notification ActionNotification) error {
+func (h *defaultActionHandler) Handle(notification ActionNotification) error {
 	if !utils.IsStringInSlice(notification.Action, Config.Actions.ExecuteOn) {
 		return errUnconfiguredAction
 	}
@@ -134,7 +134,7 @@ func (h defaultActionHandler) Handle(notification ActionNotification) error {
 	return h.handleCommand(notification)
 }
 
-func (h defaultActionHandler) handleHTTP(notification ActionNotification) error {
+func (h *defaultActionHandler) handleHTTP(notification ActionNotification) error {
 	u, err := url.Parse(Config.Actions.Hook)
 	if err != nil {
 		logger.Warn(notification.Protocol, "", "Invalid hook %#v for operation %#v: %v", Config.Actions.Hook, notification.Action, err)
@@ -145,7 +145,7 @@ func (h defaultActionHandler) handleHTTP(notification ActionNotification) error
 	startTime := time.Now()
 	respCode := 0
 
-	httpClient := httpclient.GetHTTPClient()
+	httpClient := httpclient.GetRetraybleHTTPClient()
 
 	var b bytes.Buffer
 	_ = json.NewEncoder(&b).Encode(notification)
@@ -165,7 +165,7 @@ func (h defaultActionHandler) handleHTTP(notification ActionNotification) error
 	return err
 }
 
-func (h defaultActionHandler) handleCommand(notification ActionNotification) error {
+func (h *defaultActionHandler) handleCommand(notification ActionNotification) error {
 	if !filepath.IsAbs(Config.Actions.Hook) {
 		err := fmt.Errorf("invalid notification command %#v", Config.Actions.Hook)
 		logger.Warn(notification.Protocol, "", "unable to execute notification command: %v", err)

common/actions_test.go

@@ -212,7 +212,7 @@ func TestInitializeActionHandler(t *testing.T) {
 
 	InitializeActionHandler(handler)
 	t.Cleanup(func() {
-		InitializeActionHandler(defaultActionHandler{})
+		InitializeActionHandler(&defaultActionHandler{})
 	})
 
 	err := actionHandler.Handle(ActionNotification{})

common/common.go

@@ -376,7 +376,7 @@ func (c *Configuration) ExecutePostConnectHook(ipAddr, protocol string) error {
 				ipAddr, c.PostConnectHook, err)
 			return err
 		}
-		httpClient := httpclient.GetHTTPClient()
+		httpClient := httpclient.GetRetraybleHTTPClient()
 		q := url.Query()
 		q.Add("ip", ipAddr)
 		q.Add("protocol", protocol)

config/config.go

@@ -218,6 +218,9 @@ func Init() {
 		},
 		HTTPConfig: httpclient.Config{
 			Timeout:        20,
+			RetryWaitMin:   2,
+			RetryWaitMax:   30,
+			RetryMax:       3,
 			CACertificates: nil,
 			SkipTLSVerify:  false,
 		},
@@ -848,6 +851,9 @@ func setViperDefaults() {
 	viper.SetDefault("httpd.ca_certificates", globalConf.HTTPDConfig.CACertificates)
 	viper.SetDefault("httpd.ca_revocation_lists", globalConf.HTTPDConfig.CARevocationLists)
 	viper.SetDefault("http.timeout", globalConf.HTTPConfig.Timeout)
+	viper.SetDefault("http.retry_wait_min", globalConf.HTTPConfig.RetryWaitMin)
+	viper.SetDefault("http.retry_wait_max", globalConf.HTTPConfig.RetryWaitMax)
+	viper.SetDefault("http.retry_max", globalConf.HTTPConfig.RetryMax)
 	viper.SetDefault("http.ca_certificates", globalConf.HTTPConfig.CACertificates)
 	viper.SetDefault("http.skip_tls_verify", globalConf.HTTPConfig.SkipTLSVerify)
 	viper.SetDefault("kms.secrets.url", globalConf.KMSConfig.Secrets.URL)

dataprovider/dataprovider.go

@@ -2093,7 +2093,7 @@ func ExecutePostLoginHook(user *User, loginMethod, ip, protocol string, err erro
 
 			startTime := time.Now()
 			respCode := 0
-			httpClient := httpclient.GetHTTPClient()
+			httpClient := httpclient.GetRetraybleHTTPClient()
 			resp, err := httpClient.Post(url.String(), "application/json", bytes.NewBuffer(userAsJSON))
 			if err == nil {
 				respCode = resp.StatusCode
@@ -2273,7 +2273,7 @@ func executeAction(operation string, user *User) {
 			q.Add("action", operation)
 			url.RawQuery = q.Encode()
 			startTime := time.Now()
-			httpClient := httpclient.GetHTTPClient()
+			httpClient := httpclient.GetRetraybleHTTPClient()
 			resp, err := httpClient.Post(url.String(), "application/json", bytes.NewBuffer(userAsJSON))
 			respCode := 0
 			if err == nil {

docs/custom-actions.md

@@ -46,7 +46,7 @@ If the `hook` defines an HTTP URL then this URL will be invoked as HTTP POST. Th
 - `status`, integer. 0 means a generic error occurred. 1 means no error, 2 means quota exceeded error
 - `protocol`, string. Possible values are `SSH`, `FTP`, `DAV`
 
-The HTTP request will use the global configuration for HTTP clients.
+The HTTP hook will use the global configuration for HTTP clients and will respect the retry configurations.
 
 The `actions` struct inside the "data_provider" configuration section allows you to configure actions on user add, update, delete.
 
@@ -73,4 +73,4 @@ The program must finish within 15 seconds.
 
 If the `hook` defines an HTTP URL then this URL will be invoked as HTTP POST. The action is added to the query string, for example `<hook>?action=update`, and the user is sent serialized as JSON inside the POST body with sensitive fields removed.
 
-The HTTP request will use the global configuration for HTTP clients.
+The HTTP hook will use the global configuration for HTTP clients and will respect the retry configurations.

docs/full-configuration.md

@@ -209,8 +209,11 @@ The configuration file contains the following sections:
   - `auth_user_file`, string. Path to a file used to store usernames and passwords for basic authentication. This can be an absolute path or a path relative to the config dir. We support HTTP basic authentication, and the file format must conform to the one generated using the Apache `htpasswd` tool. The supported password formats are bcrypt (`$2y$` prefix) and md5 crypt (`$apr1$` prefix). If empty, HTTP authentication is disabled. Authentication will be always disabled for the `/healthz` endpoint.
   - `certificate_file`, string. Certificate for HTTPS. This can be an absolute path or a path relative to the config dir.
   - `certificate_key_file`, string. Private key matching the above certificate. This can be an absolute path or a path relative to the config dir. If both the certificate and the private key are provided, the server will expect HTTPS connections. Certificate and key files can be reloaded on demand sending a `SIGHUP` signal on Unix based systems and a `paramchange` request to the running service on Windows.
-- **"http"**, the configuration for HTTP clients. HTTP clients are used for executing hooks such as the ones used for custom actions, external authentication and pre-login user modifications
-  - `timeout`, integer. Timeout specifies a time limit, in seconds, for requests.
+- **"http"**, the configuration for HTTP clients. HTTP clients are used for executing hooks. Some hooks use a retryable HTTP client, for these hooks you can configure the time between retries and the number of retries. Please check the hook specific documentation to understand which hooks use a retryable HTTP client.
+  - `timeout`, integer. Timeout specifies a time limit, in seconds, for requests. For requests with retries this is the timeout for a single request
+  - `retry_wait_min`, integer. Defines the minimum waiting time between attempts in seconds.
+  - `retry_wait_max`, integer. Defines the maximum waiting time between attempts in seconds. The backoff algorithm will perform exponential backoff based on the attempt number and limited by the provided minimum and maximum durations.
+  - `retry_max`, integer. Defines the maximum number of retries if the first request fails.
   - `ca_certificates`, list of strings. List of paths to extra CA certificates to trust. The paths can be absolute or relative to the config dir. Adding trusted CA certificates is a convenient way to use self-signed certificates without defeating the purpose of using TLS.
   - `skip_tls_verify`, boolean. if enabled the HTTP client accepts any TLS certificate presented by the server and any host name in that certificate. In this mode, TLS is susceptible to man-in-the-middle attacks. This should be used only for testing.
 - **kms**, configuration for the Key Management Service, more details can be found [here](./kms.md)

docs/post-connect-hook.md

@@ -23,4 +23,4 @@ If the hook defines an HTTP URL then this URL will be invoked as HTTP GET with t
 
 The connection is accepted if the HTTP response code is `200` otherwise rejected.
 
-The HTTP request will use the global configuration for HTTP clients.
+The HTTP hook will use the global configuration for HTTP clients and will respect the retry configurations.

docs/post-login-hook.md

@@ -20,7 +20,7 @@ The program must finish within 20 seconds.
 If the hook is an HTTP URL then it will be invoked as HTTP POST. The login method, the used protocol, the ip address and the status of the user are added to the query string, for example `<http_url>?login_method=password&ip=1.2.3.4&protocol=SSH&status=1`.
 The request body will contain the user serialized as JSON.
 
-The HTTP request will use the global configuration for HTTP clients.
+The HTTP hook will use the global configuration for HTTP clients and will respect the retry configurations.
 
 The `post_login_scope` supports the following configuration values:
 

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/google/wire v0.5.0 // indirect
 	github.com/grandcat/zeroconf v1.0.0
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
+	github.com/hashicorp/go-retryablehttp v0.6.8
 	github.com/jlaffaye/ftp v0.0.0-20201112195030-9aae4d151126
 	github.com/lestrrat-go/jwx v1.1.1
 	github.com/lib/pq v1.9.0

httpclient/httpclient.go

@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/hashicorp/go-retryablehttp"
+
 	"github.com/drakkan/sftpgo/logger"
 	"github.com/drakkan/sftpgo/utils"
 )
@@ -16,8 +18,14 @@ import (
 // HTTP clients are used for executing hooks such as the ones used for
 // custom actions, external authentication and pre-login user modifications
 type Config struct {
-	// Timeout specifies a time limit, in seconds, for requests
+	// Timeout specifies a time limit, in seconds, for a request
 	Timeout int64 `json:"timeout" mapstructure:"timeout"`
+	// RetryWaitMin defines the minimum waiting time between attempts in seconds
+	RetryWaitMin int `json:"retry_wait_min" mapstructure:"retry_wait_min"`
+	// RetryWaitMax defines the minimum waiting time between attempts in seconds
+	RetryWaitMax int `json:"retry_wait_max" mapstructure:"retry_wait_max"`
+	// RetryMax defines the maximum number of attempts
+	RetryMax int `json:"retry_max" mapstructure:"retry_max"`
 	// CACertificates defines extra CA certificates to trust.
 	// The paths can be absolute or relative to the config dir.
 	// Adding trusted CA certificates is a convenient way to use self-signed
@@ -29,6 +37,7 @@ type Config struct {
 	// This should be used only for testing.
 	SkipTLSVerify   bool `json:"skip_tls_verify" mapstructure:"skip_tls_verify"`
 	customTransport *http.Transport
+	tlsConfig       *tls.Config
 }
 
 const logSender = "httpclient"
@@ -49,6 +58,7 @@ func (c Config) Initialize(configDir string) {
 	}
 	customTransport.TLSClientConfig.InsecureSkipVerify = c.SkipTLSVerify
 	httpConfig.customTransport = customTransport
+	httpConfig.tlsConfig = customTransport.TLSClientConfig
 }
 
 // loadCACerts returns system cert pools and try to add the configured
@@ -90,3 +100,17 @@ func GetHTTPClient() *http.Client {
 		Transport: httpConfig.customTransport,
 	}
 }
+
+// GetRetraybleHTTPClient returns an HTTP client that retry a request on error.
+// It uses the configured retry parameters
+func GetRetraybleHTTPClient() *retryablehttp.Client {
+	client := retryablehttp.NewClient()
+	client.HTTPClient.Timeout = time.Duration(httpConfig.Timeout) * time.Second
+	client.HTTPClient.Transport.(*http.Transport).TLSClientConfig = httpConfig.tlsConfig
+	client.Logger = &logger.LeveledLogger{Sender: "RetryableHTTPClient"}
+	client.RetryWaitMin = time.Duration(httpConfig.RetryWaitMin) * time.Second
+	client.RetryWaitMax = time.Duration(httpConfig.RetryWaitMax) * time.Second
+	client.RetryMax = httpConfig.RetryMax
+
+	return client
+}

logger/logger.go

@@ -58,6 +58,56 @@ func (l *StdLoggerWrapper) Write(p []byte) (n int, err error) {
 	return
 }
 
+// LeveledLogger is a logger that accepts a message string and a variadic number of key-value pairs
+type LeveledLogger struct {
+	Sender string
+}
+
+func (l *LeveledLogger) addKeysAndValues(ev *zerolog.Event, keysAndValues ...interface{}) {
+	for i := 0; i < len(keysAndValues); {
+		if i == len(keysAndValues)-1 {
+			break
+		}
+		key, val := keysAndValues[i], keysAndValues[i+1]
+		if keyStr, ok := key.(string); ok {
+			ev.Str(keyStr, fmt.Sprintf("%v", val))
+		}
+		i += 2
+	}
+}
+
+// Error logs at error level for the specified sender
+func (l *LeveledLogger) Error(msg string, keysAndValues ...interface{}) {
+	ev := logger.Error()
+	ev.Timestamp().Str("sender", l.Sender)
+	l.addKeysAndValues(ev, keysAndValues...)
+	ev.Send()
+}
+
+// Info logs at info level for the specified sender
+func (l *LeveledLogger) Info(msg string, keysAndValues ...interface{}) {
+	ev := logger.Info()
+	ev.Timestamp().Str("sender", l.Sender)
+	l.addKeysAndValues(ev, keysAndValues...)
+	ev.Send()
+}
+
+// Debug logs at debug level for the specified sender
+func (l *LeveledLogger) Debug(msg string, keysAndValues ...interface{}) {
+	ev := logger.Debug()
+	ev.Timestamp().Str("sender", l.Sender)
+	l.addKeysAndValues(ev, keysAndValues...)
+	ev.Send()
+}
+
+// Warn logs at warn level for the specified sender
+func (l *LeveledLogger) Warn(msg string, keysAndValues ...interface{}) {
+	ev := logger.Warn()
+	ev.Timestamp().Str("sender", l.Sender)
+	l.addKeysAndValues(ev, keysAndValues...)
+	ev.Send()
+}
+
 // GetLogger get the configured logger instance
 func GetLogger() *zerolog.Logger {
 	return &logger

sftpgo.json

@@ -174,6 +174,9 @@
   },
   "http": {
     "timeout": 20,
+    "retry_wait_min": 2,
+    "retry_wait_max": 30,
+    "retry_max": 3,
     "ca_certificates": [],
     "skip_tls_verify": false
   },