github/drone
5
0
Fork 0
mirror of https://github.com/harness/drone.git synced 2025-05-09 03:52:00 +08:00
Code Issues Packages Projects Releases Wiki Activity

[Webhook] Add display_name/description/latest_execution_result to webhook (#180)

Browse Source 
This PR adds the following fields to webhooks:
- 'DisplayName' - the display name of the webhook for easier recognition in UI (no uniqueness guarantees)
- 'Description' - an (optional) description of the webhook
 - 'LatestExecutionResult' - contains the result of the latest execution of the webhook
This commit is contained in:
Johannes Batzill 2023-01-11 17:11:10 -08:00 committed by GitHub
parent 5e3837b9cf
commit a74d779dc4
13 changed files with 221 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
internal/api/controller/webhook/create.go
View File

@ -10,15 +10,18 @@ import (
"github.com/harness/gitness/internal/auth"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/check"
"github.com/harness/gitness/types/enum"
)
type CreateInput struct {
URL string `json:"url"`
Secret string `json:"secret"`
Enabled bool `json:"enabled"`
Insecure bool `json:"insecure"`
Triggers []enum.WebhookTrigger `json:"triggers"`
DisplayName string `json:"display_name"`
Description string `json:"description"`
URL string `json:"url"`
Secret string `json:"secret"`
Enabled bool `json:"enabled"`
Insecure bool `json:"insecure"`
Triggers []enum.WebhookTrigger `json:"triggers"`
}
// Create creates a new webhook.
@ -52,11 +55,14 @@ func (c *Controller) Create(
ParentType: enum.WebhookParentRepo,
// user input
URL: in.URL,
Secret: in.Secret,
Enabled: in.Enabled,
Insecure: in.Insecure,
Triggers: deduplicateTriggers(in.Triggers),
DisplayName: in.DisplayName,
Description: in.Description,
URL: in.URL,
Secret: in.Secret,
Enabled: in.Enabled,
Insecure: in.Insecure,
Triggers: deduplicateTriggers(in.Triggers),
LatestExecutionResult: nil,
}
err = c.webhookStore.Create(ctx, hook)
@ -68,6 +74,12 @@ func (c *Controller) Create(
}
func checkCreateInput(in *CreateInput, allowLoopback bool, allowPrivateNetwork bool) error {
if err := check.DisplayName(in.DisplayName); err != nil {
return err
}
if err := check.Description(in.Description); err != nil {
return err
}
if err := checkURL(in.URL, allowLoopback, allowPrivateNetwork); err != nil {
return err
}

30
internal/api/controller/webhook/update.go
View File

@ -9,15 +9,18 @@ import (
"github.com/harness/gitness/internal/auth"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/check"
"github.com/harness/gitness/types/enum"
)
type UpdateInput struct {
URL *string `json:"url"`
Secret *string `json:"secret"`
Enabled *bool `json:"enabled"`
Insecure *bool `json:"insecure"`
Triggers []enum.WebhookTrigger `json:"triggers"`
DisplayName *string `json:"display_name"`
Description *string `json:"description"`
URL *string `json:"url"`
Secret *string `json:"secret"`
Enabled *bool `json:"enabled"`
Insecure *bool `json:"insecure"`
Triggers []enum.WebhookTrigger `json:"triggers"`
}
// Update updates an existing webhook.
@ -45,6 +48,12 @@ func (c *Controller) Update(
}
// update webhook struct (only for values that are provided)
if in.DisplayName != nil {
hook.DisplayName = *in.DisplayName
}
if in.Description != nil {
hook.Description = *in.Description
}
if in.URL != nil {
hook.URL = *in.URL
}
@ -69,6 +78,16 @@ func (c *Controller) Update(
}
func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork bool) error {
if in.DisplayName != nil {
if err := check.DisplayName(*in.DisplayName); err != nil {
return err
}
}
if in.Description != nil {
if err := check.Description(*in.Description); err != nil {
return err
}
}
if in.URL != nil {
if err := checkURL(*in.URL, allowLoopback, allowPrivateNetwork); err != nil {
return err
@ -79,7 +98,6 @@ func checkUpdateInput(in *UpdateInput, allowLoopback bool, allowPrivateNetwork b
return err
}
}
if in.Triggers != nil {
if err := checkTriggers(in.Triggers); err != nil {
return err

7
internal/store/database/migrate/postgres/0007_create_table_webhooks.up.sql
View File

@ -6,11 +6,14 @@ webhook_id SERIAL PRIMARY KEY
,webhook_updated BIGINT NOT NULL
,webhook_space_id INTEGER
,webhook_repo_id INTEGER
,webhook_display_name TEXT NOT NULL
,webhook_description TEXT NOT NULL
,webhook_url TEXT NOT NULL
,webhook_secret TEXT
,webhook_secret TEXT NOT NULL
,webhook_enabled BOOLEAN NOT NULL
,webhook_insecure BOOLEAN NOT NULL
,webhook_triggers TEXT
,webhook_triggers TEXT NOT NULL
,webhook_latest_execution_result TEXT
,CONSTRAINT fk_webhook_created_by FOREIGN KEY (webhook_created_by)
REFERENCES principals (principal_id) MATCH SIMPLE
ON UPDATE NO ACTION

7
internal/store/database/migrate/sqlite/0007_create_table_webhooks.up.sql
View File

@ -6,11 +6,14 @@ webhook_id INTEGER PRIMARY KEY AUTOINCREMENT
,webhook_updated BIGINT NOT NULL
,webhook_space_id INTEGER
,webhook_repo_id INTEGER
,webhook_display_name TEXT NOT NULL
,webhook_description TEXT NOT NULL
,webhook_url TEXT NOT NULL
,webhook_secret TEXT
,webhook_secret TEXT NOT NULL
,webhook_enabled BOOLEAN NOT NULL
,webhook_insecure BOOLEAN NOT NULL
,webhook_triggers TEXT
,webhook_triggers TEXT NOT NULL
,webhook_latest_execution_result TEXT
,CONSTRAINT fk_webhook_created_by FOREIGN KEY (webhook_created_by)
REFERENCES principals (principal_id) MATCH SIMPLE
ON UPDATE NO ACTION

102
internal/store/database/webhook.go
View File

@ -17,6 +17,7 @@ import (
"github.com/guregu/null"
"github.com/jmoiron/sqlx"
"github.com/pkg/errors"
)
var _ store.WebhookStore = (*WebhookStore)(nil)
@ -44,11 +45,14 @@ type webhook struct {
Created int64 `db:"webhook_created"`
Updated int64 `db:"webhook_updated"`
URL string `db:"webhook_url"`
Secret string `db:"webhook_secret"`
Enabled bool `db:"webhook_enabled"`
Insecure bool `db:"webhook_insecure"`
Triggers string `db:"webhook_triggers"`
DisplayName string `db:"webhook_display_name"`
Description string `db:"webhook_description"`
URL string `db:"webhook_url"`
Secret string `db:"webhook_secret"`
Enabled bool `db:"webhook_enabled"`
Insecure bool `db:"webhook_insecure"`
Triggers string `db:"webhook_triggers"`
LatestExecutionResult null.String `db:"webhook_latest_execution_result"`
}
const (
@ -60,11 +64,14 @@ const (
,webhook_created_by
,webhook_created
,webhook_updated
,webhook_display_name
,webhook_description
,webhook_url
,webhook_secret
,webhook_enabled
,webhook_insecure
,webhook_triggers`
,webhook_triggers
,webhook_latest_execution_result`
webhookSelectBase = `
SELECT` + webhookColumns + `
@ -100,22 +107,28 @@ func (s *WebhookStore) Create(ctx context.Context, hook *types.Webhook) error {
,webhook_created_by
,webhook_created
,webhook_updated
,webhook_display_name
,webhook_description
,webhook_url
,webhook_secret
,webhook_enabled
,webhook_insecure
,webhook_triggers
,webhook_latest_execution_result
) values (
:webhook_repo_id
,:webhook_space_id
,:webhook_created_by
,:webhook_created
,:webhook_updated
,:webhook_display_name
,:webhook_description
,:webhook_url
,:webhook_secret
,:webhook_enabled
,:webhook_insecure
,:webhook_triggers
,:webhook_latest_execution_result
) RETURNING webhook_id`
db := dbtx.GetAccessor(ctx, s.db)
@ -142,13 +155,16 @@ func (s *WebhookStore) Update(ctx context.Context, hook *types.Webhook) error {
const sqlQuery = `
UPDATE webhooks
SET
webhook_version = :webhook_version
webhook_version = :webhook_version
,webhook_updated = :webhook_updated
,webhook_display_name = :webhook_display_name
,webhook_description = :webhook_description
,webhook_url = :webhook_url
,webhook_secret = :webhook_secret
,webhook_enabled = :webhook_enabled
,webhook_insecure = :webhook_insecure
,webhook_triggers = :webhook_triggers
,webhook_latest_execution_result = :webhook_latest_execution_result
WHERE webhook_id = :webhook_id and webhook_version = :webhook_version - 1`
db := dbtx.GetAccessor(ctx, s.db)
@ -187,6 +203,32 @@ func (s *WebhookStore) Update(ctx context.Context, hook *types.Webhook) error {
return nil
}
// UpdateOptLock updates the webhook using the optimistic locking mechanism.
func (s *WebhookStore) UpdateOptLock(ctx context.Context, hook *types.Webhook,
mutateFn func(hook *types.Webhook) error) (*types.Webhook, error) {
for {
dup := *hook
err := mutateFn(&dup)
if err != nil {
return nil, fmt.Errorf("failed to mutate the webhook: %w", err)
}
err = s.Update(ctx, &dup)
if err == nil {
return &dup, nil
}
if !errors.Is(err, store.ErrConflict) {
return nil, fmt.Errorf("failed to update the webhook: %w", err)
}
hook, err = s.Find(ctx, hook.ID)
if err != nil {
return nil, fmt.Errorf("failed to find the latst version of the webhook: %w", err)
}
}
}
// Delete deletes the webhook for the given id.
func (s *WebhookStore) Delete(ctx context.Context, id int64) error {
const sqlQuery = `
@ -276,16 +318,19 @@ func (s *WebhookStore) List(ctx context.Context, parentType enum.WebhookParent,
func mapToWebhook(hook *webhook) (*types.Webhook, error) {
res := &types.Webhook{
ID: hook.ID,
Version: hook.Version,
CreatedBy: hook.CreatedBy,
Created: hook.Created,
Updated: hook.Updated,
URL: hook.URL,
Secret: hook.Secret,
Enabled: hook.Enabled,
Insecure: hook.Insecure,
Triggers: triggersFromString(hook.Triggers),
ID: hook.ID,
Version: hook.Version,
CreatedBy: hook.CreatedBy,
Created: hook.Created,
Updated: hook.Updated,
DisplayName: hook.DisplayName,
Description: hook.Description,
URL: hook.URL,
Secret: hook.Secret,
Enabled: hook.Enabled,
Insecure: hook.Insecure,
Triggers: triggersFromString(hook.Triggers),
LatestExecutionResult: (*enum.WebhookExecutionResult)(hook.LatestExecutionResult.Ptr()),
}
switch {
@ -306,16 +351,19 @@ func mapToWebhook(hook *webhook) (*types.Webhook, error) {
func mapToInternalWebhook(hook *types.Webhook) (*webhook, error) {
res := &webhook{
ID: hook.ID,
Version: hook.Version,
CreatedBy: hook.CreatedBy,
Created: hook.Created,
Updated: hook.Updated,
URL: hook.URL,
Secret: hook.Secret,
Enabled: hook.Enabled,
Insecure: hook.Insecure,
Triggers: triggersToString(hook.Triggers),
ID: hook.ID,
Version: hook.Version,
CreatedBy: hook.CreatedBy,
Created: hook.Created,
Updated: hook.Updated,
DisplayName: hook.DisplayName,
Description: hook.Description,
URL: hook.URL,
Secret: hook.Secret,
Enabled: hook.Enabled,
Insecure: hook.Insecure,
Triggers: triggersToString(hook.Triggers),
LatestExecutionResult: null.StringFromPtr((*string)(hook.LatestExecutionResult)),
}
switch hook.ParentType {

10
internal/store/store.go
View File

@ -321,6 +321,10 @@ type (
// Update updates an existing webhook.
Update(ctx context.Context, hook *types.Webhook) error
// UpdateOptLock updates the webhook using the optimistic locking mechanism.
UpdateOptLock(ctx context.Context, hook *types.Webhook,
mutateFn func(hook *types.Webhook) error) (*types.Webhook, error)
// Delete deletes the webhook for the given id.
Delete(ctx context.Context, id int64) error
@ -348,10 +352,4 @@ type (
// ListForTrigger lists the webhook executions for a given trigger id.
ListForTrigger(ctx context.Context, triggerID string) ([]*types.WebhookExecution, error)
}
// SystemStore defines internal system metadata storage.
SystemStore interface {
// Config returns the system configuration.
Config(ctx context.Context) *types.Config
}
)

36
internal/webhook/trigger.go
View File

@ -167,6 +167,7 @@ func (s *Server) RetriggerWebhookExecution(ctx context.Context, webhookExecution
}, nil
}
//nolint:gocognit // refactor into smaller chunks if necessary.
func (s *Server) executeWebhook(ctx context.Context, webhook *types.Webhook, triggerID string,
triggerType enum.WebhookTrigger, body any, rerunOfID *int64) (*types.WebhookExecution, error) {
// build execution entry on the fly (save no matter what)
@ -188,9 +189,22 @@ func (s *Server) executeWebhook(ctx context.Context, webhook *types.Webhook, tri
err := s.webhookExecutionStore.Create(oCtx, &execution)
if err != nil {
log.Ctx(ctx).Warn().Err(err).Msgf(
"failed to store webhook execution that ended with Result: %d, Response.Status: '%s', Error: '%s'",
"failed to store webhook execution that ended with Result: %s, Response.Status: '%s', Error: '%s'",
execution.Result, execution.Response.Status, execution.Error)
}
// update latest execution result of webhook IFF it's different from before (best effort)
if webhook.LatestExecutionResult == nil || *webhook.LatestExecutionResult != execution.Result {
_, err = s.webhookStore.UpdateOptLock(oCtx, webhook, func(hook *types.Webhook) error {
hook.LatestExecutionResult = &execution.Result
return nil
})
if err != nil {
log.Ctx(ctx).Warn().Err(err).Msgf(
"failed to update latest execution result to %s for webhook %d",
execution.Result, webhook.ID)
}
}
}(ctx, time.Now())
// derive context with time limit
@ -293,12 +307,6 @@ func prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution,
execution.Request.Body = bBuff.String()
execution.Retriggerable = true
// generate HMAC
hmac, err := generateHMACSHA256(bBuff.Bytes(), []byte(webhook.Secret))
if err != nil {
return nil, fmt.Errorf("failed to generate SHA256 based HMAC: %w", err)
}
// create request (url + body)
req, err := http.NewRequestWithContext(ctx, http.MethodPost, webhook.URL, bBuff)
if err != nil {
@ -310,10 +318,20 @@ func prepareHTTPRequest(ctx context.Context, execution *types.WebhookExecution,
}
// setup headers
// TODO: Take 'Gitness' as config input?
req.Header.Add("X-Gitness-Signature", hmac)
req.Header.Add("User-Agent", fmt.Sprintf("Gitness/%s", version.Version))
req.Header.Add("Content-Type", "application/json")
// add HMAC only if a secret was provided
if webhook.Secret != "" {
var hmac string
hmac, err = generateHMACSHA256(bBuff.Bytes(), []byte(webhook.Secret))
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)
}
hBuffer := &bytes.Buffer{}
err = req.Header.Write(hBuffer)
if err != nil {

2
mocks/mock.go
View File

@ -5,5 +5,5 @@
// Package mocks provides mock interfaces.
package mocks
//go:generate mockgen -package=mocks -destination=mock_store.go github.com/harness/gitness/internal/store SystemStore,PrincipalStore,SpaceStore,RepoStore
//go:generate mockgen -package=mocks -destination=mock_store.go github.com/harness/gitness/internal/store PrincipalStore,SpaceStore,RepoStore
//go:generate mockgen -package=mocks -destination=mock_client.go github.com/harness/gitness/client Client

42
mocks/mock_store.go
View File

@ -1,5 +1,5 @@
// Code generated by MockGen. DO NOT EDIT.
// Source: github.com/harness/gitness/internal/store (interfaces: SystemStore,PrincipalStore,SpaceStore,RepoStore)
// Source: github.com/harness/gitness/internal/store (interfaces: PrincipalStore,SpaceStore,RepoStore)
// Package mocks is a generated GoMock package.
package mocks
@ -8,49 +8,11 @@ import (
context "context"
reflect "reflect"
gomock "github.com/golang/mock/gomock"
types "github.com/harness/gitness/types"
enum "github.com/harness/gitness/types/enum"
gomock "github.com/golang/mock/gomock"
)
// MockSystemStore is a mock of SystemStore interface.
type MockSystemStore struct {
ctrl *gomock.Controller
recorder *MockSystemStoreMockRecorder
}
// MockSystemStoreMockRecorder is the mock recorder for MockSystemStore.
type MockSystemStoreMockRecorder struct {
mock *MockSystemStore
}
// NewMockSystemStore creates a new mock instance.
func NewMockSystemStore(ctrl *gomock.Controller) *MockSystemStore {
mock := &MockSystemStore{ctrl: ctrl}
mock.recorder = &MockSystemStoreMockRecorder{mock}
return mock
}
// EXPECT returns an object that allows the caller to indicate expected use.
func (m *MockSystemStore) EXPECT() *MockSystemStoreMockRecorder {
return m.recorder
}
// Config mocks base method.
func (m *MockSystemStore) Config(arg0 context.Context) *types.Config {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "Config", arg0)
ret0, _ := ret[0].(*types.Config)
return ret0
}
// Config indicates an expected call of Config.
func (mr *MockSystemStoreMockRecorder) Config(arg0 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Config", reflect.TypeOf((*MockSystemStore)(nil).Config), arg0)
}
// MockPrincipalStore is a mock of PrincipalStore interface.
type MockPrincipalStore struct {
ctrl *gomock.Controller

16
types/check/common.go
View File

@ -19,6 +19,8 @@ const (
minEmailLength = 1
maxEmailLength = 250
maxDescriptionLength = 1024
)
var (
@ -26,6 +28,10 @@ var (
fmt.Sprintf("DisplayName has to be between %d and %d in length.", minDisplayNameLength, maxDisplayNameLength),
}
ErrDescriptionTooLong = &ValidationError{
fmt.Sprintf("Description can be at most %d in length.", maxDescriptionLength),
}
ErrUIDLength = &ValidationError{
fmt.Sprintf("UID has to be between %d and %d in length.",
minUIDLength, maxUIDLength),
@ -51,6 +57,16 @@ func DisplayName(displayName string) error {
return ForControlCharacters(displayName)
}
// Description checks the provided description and returns an error if it isn't valid.
func Description(description string) error {
l := len(description)
if l > maxDescriptionLength {
return ErrDescriptionTooLong
}
return ForControlCharacters(description)
}
// ForControlCharacters ensures that there are no control characters in the provided string.
func ForControlCharacters(s string) error {
for _, r := range s {

5
types/check/repo.go
View File

@ -35,6 +35,11 @@ func RepoNoUID(repo *types.Repository) error {
return ErrRepositoryRequiresParentID
}
// validate description
if err := Description(repo.Description); err != nil {
return err
}
// TODO: validate defaultBranch, ...
return nil

5
types/check/space.go
View File

@ -51,5 +51,10 @@ func SpaceNoUID(space *types.Space) error {
}
}
// validate description
if err := Description(space.Description); err != nil {
return err
}
return nil
}

37
types/webhook.go
View File

@ -5,6 +5,8 @@
package types
import (
"encoding/json"
"github.com/harness/gitness/types/enum"
)
@ -18,11 +20,32 @@ type Webhook struct {
Created int64 `json:"created"`
Updated int64 `json:"updated"`
URL string `json:"url"`
Secret string `json:"-"`
Enabled bool `json:"enabled"`
Insecure bool `json:"insecure"`
Triggers []enum.WebhookTrigger `json:"triggers"`
DisplayName string `json:"display_name"`
Description string `json:"description"`
URL string `json:"url"`
Secret string `json:"-"`
Enabled bool `json:"enabled"`
Insecure bool `json:"insecure"`
Triggers []enum.WebhookTrigger `json:"triggers"`
LatestExecutionResult *enum.WebhookExecutionResult `json:"latest_execution_result,omitempty"`
}
// MarshalJSON overrides the default json marshaling for `Webhook` allowing us to inject the `HasSecret` field.
// NOTE: This is required as we don't expose the `Secret` field and thus the caller wouldn't know whether
// the webhook contains a secret or not.
// NOTE: This is used as an alternative to adding an `HasSecret` field to Webhook itself, which would
// require us to keep `HasSecret` in sync with the `Secret` field, while `HasSecret` is not used internally at all.
func (w *Webhook) MarshalJSON() ([]byte, error) {
// WebhookAlias allows us to embed the original Webhook object (avoiding redefining all fields)
// while avoiding an infinite loop of marsheling.
type WebhookAlias Webhook
return json.Marshal(&struct {
*WebhookAlias
HasSecret bool `json:"has_secret"`
}{
WebhookAlias: (*WebhookAlias)(w),
HasSecret: w != nil && w.Secret != "",
})
}
// WebhookExecution represents a single execution of a webhook.
@ -50,8 +73,8 @@ type WebhookExecutionRequest struct {
// WebhookExecutionResponse represents the response of a webhook execution.
type WebhookExecutionResponse struct {
StatusCode int `json:"status"`
Status string `json:"status_code"`
StatusCode int `json:"status_code"`
Status string `json:"status"`
Headers string `json:"headers"`
Body string `json:"body"`
}