From 27777bb2b1fa06f8402e2ed9f25b6afd1875eddc Mon Sep 17 00:00:00 2001 From: Johannes Batzill Date: Wed, 1 Feb 2023 19:08:25 -0800 Subject: [PATCH] [Webhook] Expose User-Agent and X-{NAME} via Config, enhance request headers (#289) --- internal/services/webhook/service.go | 24 ++++++++++++++++++++++-- internal/services/webhook/trigger.go | 21 ++++++++++++++------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/internal/services/webhook/service.go b/internal/services/webhook/service.go index b6ee63a6e..6c10c11f1 100644 --- a/internal/services/webhook/service.go +++ b/internal/services/webhook/service.go @@ -25,6 +25,14 @@ const ( ) type Config struct { + // UserAgentIdentity specifies the identity used for the user agent header + // IMPORTANT: do not include version. + UserAgentIdentity string `envconfig:"GITNESS_WEBHOOK_USER_AGENT_IDENTITY" default:"Gitness"` + // HeaderIdentity specifies the identity used for headers in webhook calls (e.g. X-Gitness-Trigger, ...). + // NOTE: If no value is provided, the UserAgentIdentity will be used. + HeaderIdentity string `envconfig:"GITNESS_WEBHOOK_HEADER_IDENTITY"` + // EventReaderName is the name used to read events from stream. + // Note: this should be different for every running instance. EventReaderName string `envconfig:"GITNESS_WEBHOOK_EVENT_READER_NAME"` Concurrency int `envconfig:"GITNESS_WEBHOOK_CONCURRENCY" default:"4"` MaxRetries int `envconfig:"GITNESS_WEBHOOK_MAX_RETRIES" default:"3"` @@ -32,13 +40,16 @@ type Config struct { AllowLoopback bool `envconfig:"GITNESS_WEBHOOK_ALLOW_LOOPBACK" default:"false"` } -func (c *Config) Validate() error { +func (c *Config) Prepare() error { if c == nil { return errors.New("config is required") } if c.EventReaderName == "" { return errors.New("config.EventReaderName is required") } + if c.UserAgentIdentity == "" { + return errors.New("config.UserAgentIdentity is required") + } if c.Concurrency < 1 { return errors.New("config.Concurrency has to be a positive number") } @@ -46,6 +57,11 @@ func (c *Config) Validate() error { return errors.New("config.MaxRetries can't be negative") } + // Backfill data + if c.HeaderIdentity == "" { + c.HeaderIdentity = c.UserAgentIdentity + } + return nil } @@ -61,6 +77,8 @@ type Service struct { secureHTTPClient *http.Client insecureHTTPClient *http.Client + + config Config } func NewService(ctx context.Context, config Config, @@ -69,7 +87,7 @@ func NewService(ctx context.Context, config Config, webhookStore store.WebhookStore, webhookExecutionStore store.WebhookExecutionStore, repoStore store.RepoStore, pullreqStore store.PullReqStore, urlProvider *url.Provider, principalStore store.PrincipalStore, gitRPCClient gitrpc.Interface) (*Service, error) { - if err := config.Validate(); err != nil { + if err := config.Prepare(); err != nil { return nil, fmt.Errorf("provided config is invalid: %w", err) } service := &Service{ @@ -83,6 +101,8 @@ func NewService(ctx context.Context, config Config, secureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, false), insecureHTTPClient: newHTTPClient(config.AllowLoopback, config.AllowPrivateNetwork, true), + + config: config, } _, err := gitReaderFactory.Launch(ctx, eventsReaderGroupName, config.EventReaderName, diff --git a/internal/services/webhook/trigger.go b/internal/services/webhook/trigger.go index 2206c40c6..06ec33f3a 100644 --- a/internal/services/webhook/trigger.go +++ b/internal/services/webhook/trigger.go @@ -212,7 +212,7 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr defer cancel() // create request from webhook and body - req, err := prepareHTTPRequest(ctx, &execution, webhook, body) + req, err := s.prepareHTTPRequest(ctx, &execution, triggerType, webhook, body) if err != nil { return &execution, err } @@ -266,10 +266,10 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr } // prepareHTTPRequest prepares a new http.Request object for the webhook using the provided body as request body. -// All execution.Request.XXX values are set accordingly +// All execution.Request.XXX values are set accordingly. // NOTE: if the body is an io.Reader, the value is used as response body as is, otherwise it'll be JSON serialized. -func prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, - webhook *types.Webhook, body any) (*http.Request, error) { +func (s *Service) prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, + triggerType enum.WebhookTrigger, webhook *types.Webhook, body any) (*http.Request, error) { // set URL as is (already has been validated, any other error will be caught in request creation) execution.Request.URL = webhook.URL @@ -318,8 +318,12 @@ func prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, } // setup headers - req.Header.Add("User-Agent", fmt.Sprintf("Gitness/%s", version.Version)) + req.Header.Add("User-Agent", fmt.Sprintf("%s/%s", s.config.UserAgentIdentity, version.Version)) req.Header.Add("Content-Type", "application/json") + req.Header.Add(s.toXHeader("Trigger"), string(triggerType)) + req.Header.Add(s.toXHeader("Webhook-Id"), fmt.Sprint(webhook.ID)) + req.Header.Add(s.toXHeader("Webhook-Parent-Type"), string(webhook.ParentType)) + req.Header.Add(s.toXHeader("Webhook-Parent-Id"), fmt.Sprint(webhook.ParentID)) // add HMAC only if a secret was provided if webhook.Secret != "" { @@ -328,8 +332,7 @@ func prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, if err != nil { return nil, fmt.Errorf("failed to generate SHA256 based HMAC: %w", err) } - // TODO: Take 'Gitness' as config input? - req.Header.Add("X-Gitness-Signature", hmac) + req.Header.Add(s.toXHeader("Signature"), hmac) } hBuffer := &bytes.Buffer{} @@ -345,6 +348,10 @@ func prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution, return req, nil } +func (s *Service) toXHeader(name string) string { + return fmt.Sprintf("X-%s-%s", s.config.HeaderIdentity, name) +} + //nolint:funlen // refactor if needed func handleWebhookResponse(execution *types.WebhookExecution, resp *http.Response) error { // store status (handle status later - want to first read body)