Skip to content

Commit

Permalink
feat: clean up webhook template configuration and nest it under regul…
Browse files Browse the repository at this point in the history
…ar webhook configuration, add tests
  • Loading branch information
yquansah committed Sep 19, 2023
1 parent 72c9e15 commit d5403ba
Show file tree
Hide file tree
Showing 16 changed files with 338 additions and 198 deletions.
5 changes: 5 additions & 0 deletions config/flipt.schema.cue
Original file line number Diff line number Diff line change
Expand Up @@ -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?: {
Expand Down
18 changes: 18 additions & 0 deletions config/flipt.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
25 changes: 16 additions & 9 deletions internal/cmd/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Check warning on line 370 in internal/cmd/grpc.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/grpc.go#L360-L370

Added lines #L360 - L370 were not covered by tests
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
}
}

Check warning on line 375 in internal/cmd/grpc.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/grpc.go#L372-L375

Added lines #L372 - L375 were not covered by tests

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,
Expand Down
33 changes: 17 additions & 16 deletions internal/config/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}

Check warning on line 59 in internal/config/audit.go

View check run for this annotation

Codecov / codecov/patch

internal/config/audit.go#L57-L59

Added lines #L57 - L59 were not covered by tests
}

if c.Buffer.Capacity < 2 || c.Buffer.Capacity > 10 {
Expand All @@ -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
Expand All @@ -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"`
Expand Down
6 changes: 3 additions & 3 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
84 changes: 84 additions & 0 deletions internal/server/audit/retryable_client.go
Original file line number Diff line number Diff line change
@@ -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
}

Check warning on line 56 in internal/server/audit/retryable_client.go

View check run for this annotation

Codecov / codecov/patch

internal/server/audit/retryable_client.go#L54-L56

Added lines #L54 - L56 were not covered by tests

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

Check warning on line 68 in internal/server/audit/retryable_client.go

View check run for this annotation

Codecov / codecov/patch

internal/server/audit/retryable_client.go#L65-L68

Added lines #L65 - L68 were not covered by tests
}

_, _ = 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
}
56 changes: 56 additions & 0 deletions internal/server/audit/retryable_client_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
Loading

0 comments on commit d5403ba

Please sign in to comment.