diff --git a/config/flipt.schema.cue b/config/flipt.schema.cue index 80f3d5badf..5218ab10bf 100644 --- a/config/flipt.schema.cue +++ b/config/flipt.schema.cue @@ -232,6 +232,11 @@ import "strings" url?: string | *"" max_backoff_duration?: =~#duration | *"15s" signing_secret?: string | *"" + templates?: [...{ + url: string + body: string + headers?: [string]: string + }] } } buffer?: { diff --git a/config/flipt.schema.json b/config/flipt.schema.json index 6753bafc2f..9374cb0618 100644 --- a/config/flipt.schema.json +++ b/config/flipt.schema.json @@ -694,6 +694,24 @@ "signing_secret": { "type": "string", "default": "" + }, + "templates": { + "type": ["array", "null"], + "items": { + "type": "object", + "properties": { + "url": { + "type": "string" + }, + "body": { + "type": "string" + }, + "headers": { + "type": "object" + } + } + }, + "additionalProperties": false } } } diff --git a/go.mod b/go.mod index eee8ccf0b5..02d6b3fbf2 100644 --- a/go.mod +++ b/go.mod @@ -72,6 +72,7 @@ require ( gopkg.in/segmentio/analytics-go.v3 v3.1.0 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 + gotest.tools v2.2.0+incompatible ) require ( diff --git a/go.sum b/go.sum index 9ec47a0690..bff4083b56 100644 --- a/go.sum +++ b/go.sum @@ -1122,6 +1122,8 @@ gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gorm.io/driver/postgres v1.0.8/go.mod h1:4eOzrI1MUfm6ObJU/UcmbXyiHSs8jSwH95G5P5dxcAg= gorm.io/gorm v1.20.12/go.mod h1:0HFTzE/SqkGTzK6TlDPPQbAYCluiVvhzoA1+aVyzenw= gorm.io/gorm v1.21.4/go.mod h1:0HFTzE/SqkGTzK6TlDPPQbAYCluiVvhzoA1+aVyzenw= +gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= +gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= gotest.tools/v3 v3.5.0 h1:Ljk6PdHdOhAb5aDMWXjDLMMhph+BpztA4v1QdqEW2eY= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go index ec518fe8e4..f5f43c6c64 100644 --- a/internal/cmd/grpc.go +++ b/internal/cmd/grpc.go @@ -356,18 +356,25 @@ func NewGRPCServer( opts = append(opts, webhook.WithMaxBackoffDuration(cfg.Audit.Sinks.Webhook.MaxBackoffDuration)) } - webhookSink := webhook.NewSink(logger, webhook.NewHTTPClient(logger, cfg.Audit.Sinks.Webhook.URL, cfg.Audit.Sinks.Webhook.SigningSecret, opts...)) - - sinks = append(sinks, webhookSink) - } + var webhookSink audit.Sink + + // Enable basic webhook sink if URL is non-empty, otherwise enable template sink if the length of templates is greater + // than 0 for the webhook. + if cfg.Audit.Sinks.Webhook.URL != "" { + webhookSink = webhook.NewSink(logger, webhook.NewWebhookClient(logger, cfg.Audit.Sinks.Webhook.URL, cfg.Audit.Sinks.Webhook.SigningSecret, opts...)) + } else if len(cfg.Audit.Sinks.Webhook.Templates) > 0 { + maxBackoffDuration := 15 * time.Second + if cfg.Audit.Sinks.Webhook.MaxBackoffDuration != 0 { + maxBackoffDuration = cfg.Audit.Sinks.Webhook.MaxBackoffDuration + } - if cfg.Audit.Sinks.WebhookTemplate.Enabled { - webhookTemplateSink, err := template.NewSink(logger, cfg.Audit.Sinks.WebhookTemplate.Templates) - if err != nil { - return nil, err + webhookSink, err = template.NewSink(logger, cfg.Audit.Sinks.Webhook.Templates, maxBackoffDuration) + if err != nil { + return nil, err + } } - sinks = append(sinks, webhookTemplateSink) + sinks = append(sinks, webhookSink) } // based on audit sink configuration from the user, provision the audit sinks and add them to a slice, diff --git a/internal/config/audit.go b/internal/config/audit.go index e891cdcdf1..60f6dc02c4 100644 --- a/internal/config/audit.go +++ b/internal/config/audit.go @@ -20,7 +20,7 @@ type AuditConfig struct { // Enabled returns true if any nested sink is enabled func (c *AuditConfig) Enabled() bool { - return c.Sinks.LogFile.Enabled || c.Sinks.Webhook.Enabled || c.Sinks.WebhookTemplate.Enabled + return c.Sinks.LogFile.Enabled || c.Sinks.Webhook.Enabled } func (c *AuditConfig) setDefaults(v *viper.Viper) error { @@ -49,8 +49,14 @@ func (c *AuditConfig) validate() error { return errors.New("file not specified") } - if c.Sinks.Webhook.Enabled && c.Sinks.Webhook.URL == "" { - return errors.New("url not provided") + if c.Sinks.Webhook.Enabled { + if c.Sinks.Webhook.URL == "" && len(c.Sinks.Webhook.Templates) == 0 { + return errors.New("url or templates not provided") + } + + if c.Sinks.Webhook.URL != "" && len(c.Sinks.Webhook.Templates) > 0 { + return errors.New("url and templates both provided") + } } if c.Buffer.Capacity < 2 || c.Buffer.Capacity > 10 { @@ -67,18 +73,18 @@ func (c *AuditConfig) validate() error { // SinksConfig contains configuration held in structures for the different sinks // that we will send audits to. type SinksConfig struct { - LogFile LogFileSinkConfig `json:"log,omitempty" mapstructure:"log"` - Webhook WebhookSinkConfig `json:"webhook,omitempty" mapstructure:"webhook"` - WebhookTemplate WebhookTemplateConfig `json:"webhookTemplate,omitempty" mapstructure:"webhook_template"` + LogFile LogFileSinkConfig `json:"log,omitempty" mapstructure:"log"` + Webhook WebhookSinkConfig `json:"webhook,omitempty" mapstructure:"webhook"` } // WebhookSinkConfig contains configuration for sending POST requests to specific // URL as its configured. type WebhookSinkConfig struct { - Enabled bool `json:"enabled,omitempty" mapstructure:"enabled"` - URL string `json:"url,omitempty" mapstructure:"url"` - MaxBackoffDuration time.Duration `json:"maxBackoffDuration,omitempty" mapstructure:"max_backoff_duration"` - SigningSecret string `json:"signingSecret,omitempty" mapstructure:"signing_secret"` + Enabled bool `json:"enabled,omitempty" mapstructure:"enabled"` + URL string `json:"url,omitempty" mapstructure:"url"` + MaxBackoffDuration time.Duration `json:"maxBackoffDuration,omitempty" mapstructure:"max_backoff_duration"` + SigningSecret string `json:"signingSecret,omitempty" mapstructure:"signing_secret"` + Templates []WebhookTemplate `json:"templates,omitempty" mapstructure:"templates"` } // LogFileSinkConfig contains fields that hold configuration for sending audits @@ -95,13 +101,8 @@ type BufferConfig struct { FlushPeriod time.Duration `json:"flushPeriod,omitempty" mapstructure:"flush_period"` } -type WebhookTemplateConfig struct { - Enabled bool `json:"enabled,omitempty" mapstructure:"enabled"` - Templates []WebhookTemplate `json:"templates,omitempty" mapstructure:"templates"` -} - +// WebhookTemplate specifies configuration for a user to send a payload a particular destination URL. type WebhookTemplate struct { - Method string `json:"method,omitempty" mapstructure:"method"` URL string `json:"url,omitempty" mapstructure:"url"` Body string `json:"body,omitempty" mapstructure:"body"` Headers map[string]string `json:"headers,omitempty" mapstructure:"headers"` diff --git a/internal/config/config_test.go b/internal/config/config_test.go index aeb5219a04..05c5646c64 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -635,9 +635,9 @@ func TestLoad(t *testing.T) { wantErr: errors.New("file not specified"), }, { - name: "url not specified", - path: "./testdata/audit/invalid_webhook_url_not_provided.yml", - wantErr: errors.New("url not provided"), + name: "url or template not specified", + path: "./testdata/audit/invalid_webhook_url_or_template_not_provided.yml", + wantErr: errors.New("url or templates not provided"), }, { name: "local config provided", diff --git a/internal/config/testdata/audit/invalid_webhook_url_not_provided.yml b/internal/config/testdata/audit/invalid_webhook_url_or_template_not_provided.yml similarity index 100% rename from internal/config/testdata/audit/invalid_webhook_url_not_provided.yml rename to internal/config/testdata/audit/invalid_webhook_url_or_template_not_provided.yml diff --git a/internal/server/audit/retryable_client.go b/internal/server/audit/retryable_client.go new file mode 100644 index 0000000000..7c5bec437a --- /dev/null +++ b/internal/server/audit/retryable_client.go @@ -0,0 +1,84 @@ +package audit + +import ( + "context" + "errors" + "io" + "net/http" + "time" + + "github.com/cenkalti/backoff/v4" + "go.uber.org/zap" +) + +type retryClient struct { + logger *zap.Logger + *http.Client + maxBackoffDuration time.Duration +} + +// Retrier contains a minimal API for retrying a request onto an endpoint upon failure or non-200 status. +type Retrier interface { + RequestRetry(ctx context.Context, body []byte, fn RequestCreator) error +} + +// NewRetrier is a constructor for a retryClient, but returns a contract for the consumer +// to just execute retryable HTTP requests. +func NewRetrier(logger *zap.Logger, maxBackoffDuration time.Duration) Retrier { + return &retryClient{ + logger: logger, + Client: &http.Client{ + Timeout: 5 * time.Second, + }, + maxBackoffDuration: maxBackoffDuration, + } +} + +// RequestCreator provides a basic function for rewinding a request with a body, upon +// request failure. +type RequestCreator func(ctx context.Context, body []byte) (*http.Request, error) + +func (r *retryClient) RequestRetry(ctx context.Context, body []byte, fn RequestCreator) error { + be := backoff.NewExponentialBackOff() + be.MaxElapsedTime = r.maxBackoffDuration + + ticker := backoff.NewTicker(be) + defer ticker.Stop() + + successfulRequest := false + + // Make requests with configured retries. + for range ticker.C { + req, err := fn(ctx, body) + if err != nil { + r.logger.Error("error creating HTTP request", zap.Error(err)) + return err + } + + resp, err := r.Client.Do(req) + if err != nil { + r.logger.Debug("webhook request failed, retrying...", zap.Error(err)) + continue + } + + if resp.StatusCode != http.StatusOK { + r.logger.Debug("webhook request failed, retrying...", zap.Int("status_code", resp.StatusCode)) + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + continue + } + + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + + r.logger.Debug("successful request to webhook") + successfulRequest = true + break + } + + if !successfulRequest { + return errors.New("failed to send event to webhook") + } + + return nil +} diff --git a/internal/server/audit/retryable_client_test.go b/internal/server/audit/retryable_client_test.go new file mode 100644 index 0000000000..a72c71268c --- /dev/null +++ b/internal/server/audit/retryable_client_test.go @@ -0,0 +1,56 @@ +package audit + +import ( + "bytes" + "context" + "net/http" + "testing" + "time" + + "github.com/h2non/gock" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +func TestRetrier_Failure(t *testing.T) { + retrier := NewRetrier(zap.NewNop(), 5*time.Second) + + gock.New("https://respond.io"). + MatchHeader("Content-Type", "application/json"). + Post("/webhook"). + Reply(500) + defer gock.Off() + + rc := func(ctx context.Context, body []byte) (*http.Request, error) { + return http.NewRequestWithContext(ctx, http.MethodPost, "https://respond.io/webhook", bytes.NewBuffer(body)) + } + + err := retrier.RequestRetry(context.TODO(), []byte(`{"hello": "world"}`), rc) + + assert.EqualError(t, err, "failed to send event to webhook") +} + +func TestRetrier_Success(t *testing.T) { + retrier := NewRetrier(zap.NewNop(), 5*time.Second) + + gock.New("https://respond.io"). + MatchHeader("Content-Type", "application/json"). + Post("/webhook"). + Reply(200) + defer gock.Off() + + rc := func(ctx context.Context, body []byte) (*http.Request, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://respond.io/webhook", bytes.NewBuffer(body)) + if err != nil { + return nil, err + } + + req.Header.Add("Content-Type", "application/json") + + return req, nil + } + + err := retrier.RequestRetry(context.TODO(), []byte(`{"hello": "world"}`), rc) + + assert.Nil(t, err) +} diff --git a/internal/server/audit/template/webhook_template.go b/internal/server/audit/template/executer.go similarity index 52% rename from internal/server/audit/template/webhook_template.go rename to internal/server/audit/template/executer.go index 2eef17cf64..e1ad0b1415 100644 --- a/internal/server/audit/template/webhook_template.go +++ b/internal/server/audit/template/executer.go @@ -3,35 +3,32 @@ package template import ( "bytes" "context" - "fmt" - "io" "net/http" "strings" "text/template" "time" - "github.com/cenkalti/backoff/v4" "go.flipt.io/flipt/internal/server/audit" "go.uber.org/zap" ) -// WebhookTemplate contains fields that pertain to constructing an HTTP request. -type WebhookTemplate struct { - logger *zap.Logger - - url string +// webhookTemplate contains fields that pertain to constructing an HTTP request. +type webhookTemplate struct { method string + url string headers map[string]string bodyTemplate *template.Template maxBackoffDuration time.Duration + + retryableClient audit.Retrier } // Executer will try and send the event to the configured URL with all of the parameters. -// It will send the request with retries, according to the backoff strategy. +// It will send the request with the retyrable client. type Executer interface { - Execute(context.Context, *http.Client, audit.Event) error + Execute(context.Context, audit.Event) error } // NewWebhookTemplate is the constructor for a WebhookTemplate. @@ -41,17 +38,17 @@ func NewWebhookTemplate(logger *zap.Logger, method, url, body string, headers ma return nil, err } - return &WebhookTemplate{ - logger: logger, + return &webhookTemplate{ url: url, method: method, bodyTemplate: tmpl, headers: headers, maxBackoffDuration: maxBackoffDuration, + retryableClient: audit.NewRetrier(logger, maxBackoffDuration), }, nil } -func (w *WebhookTemplate) createRequest(ctx context.Context, body []byte) (*http.Request, error) { +func (w *webhookTemplate) createRequest(ctx context.Context, body []byte) (*http.Request, error) { req, err := http.NewRequestWithContext(ctx, w.method, w.url, bytes.NewBuffer(body)) if err != nil { return nil, err @@ -72,7 +69,7 @@ func (w *WebhookTemplate) createRequest(ctx context.Context, body []byte) (*http // Execute will take in an HTTP client, and the event to send upstream to a destination. It will house the logic // of doing the exponential backoff for sending the event upon failure to do so. -func (w *WebhookTemplate) Execute(ctx context.Context, client *http.Client, event audit.Event) error { +func (w *webhookTemplate) Execute(ctx context.Context, event audit.Event) error { buf := &bytes.Buffer{} err := w.bodyTemplate.Execute(buf, event) @@ -80,45 +77,9 @@ func (w *WebhookTemplate) Execute(ctx context.Context, client *http.Client, even return err } - be := backoff.NewExponentialBackOff() - be.MaxElapsedTime = w.maxBackoffDuration - - ticker := backoff.NewTicker(be) - defer ticker.Stop() - - successfulRequest := false - - // Make requests with configured retries. - for range ticker.C { - req, err := w.createRequest(ctx, buf.Bytes()) - if err != nil { - w.logger.Error("error creating HTTP request", zap.Error(err)) - return err - } - - resp, err := client.Do(req) - if err != nil { - w.logger.Debug("webhook request failed, retrying...", zap.Error(err)) - continue - } - - if resp.StatusCode != http.StatusOK { - w.logger.Debug("webhook request failed, retrying...", zap.Int("status_code", resp.StatusCode)) - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - continue - } - - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - - w.logger.Debug("successful request to webhook") - successfulRequest = true - break - } - - if !successfulRequest { - return fmt.Errorf("failed to send event to webhook url: %s", w.url) + err = w.retryableClient.RequestRetry(ctx, buf.Bytes(), w.createRequest) + if err != nil { + return err } return nil diff --git a/internal/server/audit/template/executer_test.go b/internal/server/audit/template/executer_test.go new file mode 100644 index 0000000000..4d2cff8430 --- /dev/null +++ b/internal/server/audit/template/executer_test.go @@ -0,0 +1,40 @@ +package template + +import ( + "context" + "testing" + "text/template" + "time" + + "github.com/stretchr/testify/require" + "go.flipt.io/flipt/internal/server/audit" +) + +type dummyRetrier struct{} + +func (d *dummyRetrier) RequestRetry(_ context.Context, _ []byte, _ audit.RequestCreator) error { + return nil +} + +func TestExecuter(t *testing.T) { + tmpl, err := template.New("").Parse(`{ + "type": "{{ .Type }}", + "action": "{{ .Action }}" + }`) + + require.NoError(t, err) + + whTemplate := &webhookTemplate{ + url: "https://flipt-webhook.io/webhook", + maxBackoffDuration: 15 * time.Second, + retryableClient: &dummyRetrier{}, + bodyTemplate: tmpl, + } + + err = whTemplate.Execute(context.TODO(), audit.Event{ + Type: audit.FlagType, + Action: audit.Create, + }) + + require.NoError(t, err) +} diff --git a/internal/server/audit/template/template.go b/internal/server/audit/template/template.go index 5ba66dd7a0..ee821e5201 100644 --- a/internal/server/audit/template/template.go +++ b/internal/server/audit/template/template.go @@ -12,22 +12,20 @@ import ( "go.uber.org/zap" ) -const sinkType = "webhookTemplate" +const sinkType = "templates" type Sink struct { logger *zap.Logger - *http.Client - executers []Executer } // NewSink is the constructor for a Sink. -func NewSink(logger *zap.Logger, webhookTemplates []config.WebhookTemplate) (audit.Sink, error) { +func NewSink(logger *zap.Logger, webhookTemplates []config.WebhookTemplate, maxBackoffDuration time.Duration) (audit.Sink, error) { executers := make([]Executer, 0, len(webhookTemplates)) for _, wht := range webhookTemplates { - executer, err := NewWebhookTemplate(logger, wht.Method, wht.URL, wht.Body, wht.Headers, 15*time.Second) + executer, err := NewWebhookTemplate(logger, http.MethodPost, wht.URL, wht.Body, wht.Headers, maxBackoffDuration) if err != nil { return nil, fmt.Errorf("failed to create webhook template sink: %w", err) } @@ -36,10 +34,7 @@ func NewSink(logger *zap.Logger, webhookTemplates []config.WebhookTemplate) (aud } return &Sink{ - logger: logger, - Client: &http.Client{ - Timeout: 5 * time.Second, - }, + logger: logger, executers: executers, }, nil } @@ -49,7 +44,7 @@ func (t *Sink) SendAudits(ctx context.Context, events []audit.Event) error { for _, e := range events { for _, executer := range t.executers { - err := executer.Execute(ctx, t.Client, e) + err := executer.Execute(ctx, e) if err != nil { t.logger.Error("failed to send audit to webhook", zap.Error(err)) result = multierror.Append(result, err) diff --git a/internal/server/audit/template/template_test.go b/internal/server/audit/template/template_test.go new file mode 100644 index 0000000000..64b3e5d53c --- /dev/null +++ b/internal/server/audit/template/template_test.go @@ -0,0 +1,42 @@ +package template + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "go.flipt.io/flipt/internal/server/audit" + "go.uber.org/zap" + "gotest.tools/assert" +) + +type dummyExecuter struct{} + +func (d *dummyExecuter) Execute(_ context.Context, _ audit.Event) error { + return nil +} + +func TestSink(t *testing.T) { + var s audit.Sink = &Sink{ + logger: zap.NewNop(), + executers: []Executer{&dummyExecuter{}}, + } + + assert.Equal(t, "templates", s.String()) + + err := s.SendAudits(context.TODO(), []audit.Event{ + { + Version: "0.1", + Type: audit.FlagType, + Action: audit.Create, + }, + { + Version: "0.1", + Type: audit.ConstraintType, + Action: audit.Update, + }, + }) + + require.NoError(t, err) + require.NoError(t, s.Close()) +} diff --git a/internal/server/audit/webhook/client.go b/internal/server/audit/webhook/client.go index 0585ca5a50..2348215333 100644 --- a/internal/server/audit/webhook/client.go +++ b/internal/server/audit/webhook/client.go @@ -7,11 +7,9 @@ import ( "crypto/sha256" "encoding/json" "fmt" - "io" "net/http" "time" - "github.com/cenkalti/backoff/v4" "go.flipt.io/flipt/internal/server/audit" "go.uber.org/zap" ) @@ -20,32 +18,28 @@ const ( fliptSignatureHeader = "x-flipt-webhook-signature" ) -// HTTPClient allows for sending the event payload to a configured webhook service. -type HTTPClient struct { - logger *zap.Logger - *http.Client +// webhookClient allows for sending the event payload to a configured webhook service. +type webhookClient struct { + logger *zap.Logger url string signingSecret string maxBackoffDuration time.Duration + + retryableClient audit.Retrier } // signPayload takes in a marshalled payload and returns the sha256 hash of it. -func (w *HTTPClient) signPayload(payload []byte) []byte { +func (w *webhookClient) signPayload(payload []byte) []byte { h := hmac.New(sha256.New, []byte(w.signingSecret)) h.Write(payload) return h.Sum(nil) } // NewHTTPClient is the constructor for a HTTPClient. -func NewHTTPClient(logger *zap.Logger, url, signingSecret string, opts ...ClientOption) *HTTPClient { - c := &http.Client{ - Timeout: 5 * time.Second, - } - - h := &HTTPClient{ +func NewWebhookClient(logger *zap.Logger, url, signingSecret string, opts ...ClientOption) *webhookClient { + h := &webhookClient{ logger: logger, - Client: c, url: url, signingSecret: signingSecret, maxBackoffDuration: 15 * time.Second, @@ -55,19 +49,23 @@ func NewHTTPClient(logger *zap.Logger, url, signingSecret string, opts ...Client o(h) } + retyableClient := audit.NewRetrier(logger, h.maxBackoffDuration) + + h.retryableClient = retyableClient + return h } -type ClientOption func(h *HTTPClient) +type ClientOption func(h *webhookClient) // WithMaxBackoffDuration allows for a configurable backoff duration configuration. func WithMaxBackoffDuration(maxBackoffDuration time.Duration) ClientOption { - return func(h *HTTPClient) { + return func(h *webhookClient) { h.maxBackoffDuration = maxBackoffDuration } } -func (w *HTTPClient) createRequest(ctx context.Context, body []byte) (*http.Request, error) { +func (w *webhookClient) createRequest(ctx context.Context, body []byte) (*http.Request, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, w.url, bytes.NewBuffer(body)) if err != nil { return nil, err @@ -86,51 +84,15 @@ func (w *HTTPClient) createRequest(ctx context.Context, body []byte) (*http.Requ } // SendAudit will send an audit event to a configured server at a URL. -func (w *HTTPClient) SendAudit(ctx context.Context, e audit.Event) error { +func (w *webhookClient) SendAudit(ctx context.Context, e audit.Event) error { body, err := json.Marshal(e) if err != nil { return err } - be := backoff.NewExponentialBackOff() - be.MaxElapsedTime = w.maxBackoffDuration - - ticker := backoff.NewTicker(be) - defer ticker.Stop() - - successfulRequest := false - - // Make requests with configured retries. - for range ticker.C { - req, err := w.createRequest(ctx, body) - if err != nil { - w.logger.Error("error creating HTTP request", zap.Error(err)) - return err - } - - resp, err := w.Client.Do(req) - if err != nil { - w.logger.Debug("webhook request failed, retrying...", zap.Error(err)) - continue - } - - if resp.StatusCode != http.StatusOK { - w.logger.Debug("webhook request failed, retrying...", zap.Int("status_code", resp.StatusCode)) - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - continue - } - - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - - w.logger.Debug("successful request to webhook") - successfulRequest = true - break - } - - if !successfulRequest { - return fmt.Errorf("failed to send event to webhook url: %s after %s", w.url, w.maxBackoffDuration) + err = w.retryableClient.RequestRetry(ctx, body, w.createRequest) + if err != nil { + return err } return nil diff --git a/internal/server/audit/webhook/client_test.go b/internal/server/audit/webhook/client_test.go index 56090d22be..0919b2b5e0 100644 --- a/internal/server/audit/webhook/client_test.go +++ b/internal/server/audit/webhook/client_test.go @@ -5,63 +5,29 @@ import ( "testing" "time" - "github.com/h2non/gock" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.flipt.io/flipt/internal/server/audit" "go.uber.org/zap" ) -func TestHTTPClient_Failure(t *testing.T) { - hclient := NewHTTPClient(zap.NewNop(), "https://respond.io/webhook", "", WithMaxBackoffDuration(5*time.Second)) +type dummyRetrier struct{} - gock.New("https://respond.io"). - MatchHeader("Content-Type", "application/json"). - Post("/webhook"). - Reply(500) - defer gock.Off() - - err := hclient.SendAudit(context.TODO(), audit.Event{ - Version: "0.1", - Type: audit.FlagType, - Action: audit.Create, - }) - - assert.EqualError(t, err, "failed to send event to webhook url: https://respond.io/webhook after 5s") +func (d *dummyRetrier) RequestRetry(_ context.Context, _ []byte, _ audit.RequestCreator) error { + return nil } -func TestHTTPClient_Success(t *testing.T) { - hclient := NewHTTPClient(zap.NewNop(), "https://respond.io/webhook", "", WithMaxBackoffDuration(5*time.Second)) - - gock.New("https://respond.io"). - MatchHeader("Content-Type", "application/json"). - Post("/webhook"). - Reply(200) - defer gock.Off() - - err := hclient.SendAudit(context.TODO(), audit.Event{ - Version: "0.1", - Type: audit.FlagType, - Action: audit.Create, - }) - - assert.Nil(t, err) -} - -func TestHTTPClient_Success_WithSignedPayload(t *testing.T) { - hclient := NewHTTPClient(zap.NewNop(), "https://respond.io/webhook", "supersecret", WithMaxBackoffDuration(5*time.Second)) - - gock.New("https://respond.io"). - MatchHeader("Content-Type", "application/json"). - MatchHeader(fliptSignatureHeader, "daae3795b8b2762be113870086d6d04ea3618b90ff14925fe4caaa1425638e4f"). - Post("/webhook"). - Reply(200) - defer gock.Off() - - err := hclient.SendAudit(context.TODO(), audit.Event{ - Version: "0.1", - Type: audit.FlagType, - Action: audit.Create, +func TestWebhookClient(t *testing.T) { + whclient := &webhookClient{ + logger: zap.NewNop(), + url: "https://flipt-webhook.io/webhook", + maxBackoffDuration: 15 * time.Second, + retryableClient: &dummyRetrier{}, + } + + err := whclient.SendAudit(context.TODO(), audit.Event{ + Type: audit.FlagType, + Action: audit.Create, }) - assert.Nil(t, err) + require.NoError(t, err) }