Add retries to kibana client

Author: jsoriano

go.mod

@@ -24,6 +24,7 @@ require (
 	github.com/google/go-github/v32 v32.1.0
 	github.com/google/go-querystring v1.1.0
 	github.com/google/uuid v1.4.0
+	github.com/hashicorp/go-retryablehttp v0.7.5
 	github.com/jedib0t/go-pretty v4.3.0+incompatible
 	github.com/magefile/mage v1.15.0
 	github.com/mholt/archiver/v3 v3.5.1
@@ -93,6 +94,7 @@ require (
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
 	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
+	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/huandu/xstrings v1.4.0 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect

go.sum

@@ -301,8 +301,14 @@ github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79/go.mod h1:Fecb
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
 github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
+github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
+github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
+github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
+github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
 github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
 github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
+github.com/hashicorp/go-retryablehttp v0.7.5 h1:bJj+Pj19UZMIweq/iie+1u5YCdGrnxCT9yvm0e+Nd5M=
+github.com/hashicorp/go-retryablehttp v0.7.5/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8=
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec h1:qv2VnGeEQHchGaZ/u7lxST/RaJw+cv273q79D81Xbog=

internal/kibana/client.go

@@ -18,6 +18,7 @@ import (
 	"github.com/elastic/elastic-package/internal/certs"
 	"github.com/elastic/elastic-package/internal/install"
 	"github.com/elastic/elastic-package/internal/logger"
+	"github.com/elastic/elastic-package/internal/retry"
 )
 
 var ErrUndefinedHost = errors.New("missing kibana host")
@@ -33,14 +34,19 @@ type Client struct {
 
 	versionInfo VersionInfo
 	semver      *semver.Version
+
+	retryMax int
+	http     *http.Client
 }
 
 // ClientOption is functional option modifying Kibana client.
 type ClientOption func(*Client)
 
 // NewClient creates a new instance of the client.
 func NewClient(opts ...ClientOption) (*Client, error) {
-	c := &Client{}
+	c := &Client{
+		retryMax: 10,
+	}
 	for _, opt := range opts {
 		opt(c)
 	}
@@ -49,6 +55,12 @@ func NewClient(opts ...ClientOption) (*Client, error) {
 		return nil, ErrUndefinedHost
 	}
 
+	httpClient, err := c.newHttpClient()
+	if err != nil {
+		return nil, err
+	}
+	c.http = httpClient
+
 	// Allow to initialize version from tests.
 	var zeroVersion VersionInfo
 	if c.semver == nil || c.versionInfo == zeroVersion {
@@ -95,6 +107,13 @@ func Password(password string) ClientOption {
 	}
 }
 
+// RetryMax configures the number of retries before failing.
+func RetryMax(retryMax int) ClientOption {
+	return func(c *Client) {
+		c.retryMax = retryMax
+	}
+}
+
 // CertificateAuthority sets the certificate authority to be used by the client.
 func CertificateAuthority(certificateAuthority string) ClientOption {
 	return func(c *Client) {
@@ -156,31 +175,42 @@ func (c *Client) newRequest(method, resourcePath string, reqBody io.Reader) (*ht
 }
 
 func (c *Client) doRequest(request *http.Request) (int, []byte, error) {
-	client := http.Client{}
+	resp, err := c.http.Do(request)
+	if err != nil {
+		return 0, nil, fmt.Errorf("could not send request to Kibana API: %w", err)
+	}
+
+	defer resp.Body.Close()
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return resp.StatusCode, nil, fmt.Errorf("could not read response body: %w", err)
+	}
+
+	return resp.StatusCode, body, nil
+}
+
+func (c *Client) newHttpClient() (*http.Client, error) {
+	client := &http.Client{}
 	if c.tlSkipVerify {
 		client.Transport = &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 		}
 	} else if c.certificateAuthority != "" {
 		rootCAs, err := certs.SystemPoolWithCACertificate(c.certificateAuthority)
 		if err != nil {
-			return 0, nil, fmt.Errorf("reading CA certificate: %w", err)
+			return nil, fmt.Errorf("reading CA certificate: %w", err)
 		}
 		client.Transport = &http.Transport{
 			TLSClientConfig: &tls.Config{RootCAs: rootCAs},
 		}
 	}
 
-	resp, err := client.Do(request)
-	if err != nil {
-		return 0, nil, fmt.Errorf("could not send request to Kibana API: %w", err)
-	}
-
-	defer resp.Body.Close()
-	body, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return resp.StatusCode, nil, fmt.Errorf("could not read response body: %w", err)
+	if c.retryMax > 0 {
+		opts := retry.HTTPOptions{
+			RetryMax: c.retryMax,
+		}
+		client = retry.WrapHTTPClient(client, opts)
 	}
 
-	return resp.StatusCode, body, nil
+	return client, nil
 }

internal/retry/http.go

@@ -0,0 +1,126 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package retry
+
+import (
+	"context"
+	"crypto/x509"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/url"
+	"time"
+
+	"github.com/hashicorp/go-retryablehttp"
+)
+
+const (
+	defaultRetryWaitMin = 1 * time.Second
+	defaultRetryWaitMax = 5 * time.Second
+)
+
+type HTTPOptions struct {
+	RetryMax int
+
+	retryWaitMin time.Duration
+	retryWaitMax time.Duration
+}
+
+func WrapHTTPClient(client *http.Client, opts HTTPOptions) *http.Client {
+	if opts.RetryMax <= 0 {
+		return client
+	}
+	retryWaitMin := opts.retryWaitMin
+	if retryWaitMin == 0 {
+		retryWaitMin = defaultRetryWaitMin
+	}
+	retryWaitMax := opts.retryWaitMax
+	if retryWaitMax == 0 {
+		retryWaitMax = defaultRetryWaitMax
+	}
+
+	if client == nil {
+		client = &http.Client{}
+	}
+	if client.CheckRedirect == nil {
+		client.CheckRedirect = checkRedirect
+	}
+	retryClient := retryablehttp.NewClient()
+	retryClient.HTTPClient = client
+	retryClient.CheckRetry = checkRetry
+	retryClient.ErrorHandler = retryablehttp.PassthroughErrorHandler
+	retryClient.RetryMax = opts.RetryMax
+	retryClient.RetryWaitMin = retryWaitMin
+	retryClient.RetryWaitMax = retryWaitMax
+	return retryClient.StandardClient()
+}
+
+var (
+	maxRedirects   = 10
+	redirectsError = fmt.Errorf("stopped after %d redirects", maxRedirects)
+)
+
+// checkRedirect reimplements default http redirect policy but returning a typed error.
+func checkRedirect(req *http.Request, via []*http.Request) error {
+	if len(via) >= maxRedirects {
+		return redirectsError
+	}
+	return nil
+}
+
+// checkRetry reimplements retryablehttp.DefaultRetryPolicy with better error checking.
+func checkRetry(ctx context.Context, resp *http.Response, err error) (bool, error) {
+	if ctx.Err() != nil {
+		return false, ctx.Err()
+	}
+
+	if err != nil {
+		if errors.Is(err, redirectsError) {
+			// Too many redirects, let's stop here.
+			return false, nil
+		}
+
+		var urlError *url.Error
+		if errors.As(err, &urlError) {
+			// URL is invalid, not recoverable.
+			return false, nil
+		}
+
+		var certError *x509.CertificateInvalidError
+		if errors.As(err, &certError) {
+			// Invalid certificate, not recoverable.
+			return false, nil
+		}
+
+		var caError *x509.UnknownAuthorityError
+		if errors.As(err, &caError) {
+			// Unknown CA, not recoverable.
+			return false, nil
+		}
+
+		// Consider other errors as recoverable.
+		return true, nil
+	}
+
+	// 429 Too Many Requests is recoverable. Sometimes the server puts
+	// a Retry-After response header to indicate when the server is
+	// available to start processing request from client.
+	if resp.StatusCode == http.StatusTooManyRequests {
+		return true, nil
+	}
+
+	// Check the response code. We retry on 500-range responses to allow
+	// the server time to recover, as 500's are typically not permanent
+	// errors and may relate to outages on the server side. This will catch
+	// invalid response codes as well, like 0 and 999.
+	if resp.StatusCode == 0 || (resp.StatusCode >= 500 && resp.StatusCode != http.StatusNotImplemented) {
+		// Return the underlying error, that will probably be nil.
+		// retryablehttp.DefaultRetryPolicy did generate an error for these cases,
+		// but this is not what the default HTTP client does.
+		return true, err
+	}
+
+	return false, nil
+}