From bd9d15500c18e3af726bde6c2dc00d3e58dc04c6 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Thu, 5 Dec 2024 02:01:08 +0000 Subject: [PATCH] feat: [CODE-2885]: support for webhook type (#3098) --- app/api/controller/webhook/preprocessor.go | 12 ++++---- app/api/controller/webhook/repo_create.go | 4 +-- app/api/controller/webhook/repo_update.go | 4 +-- app/api/controller/webhook/space_update.go | 4 +-- app/services/webhook/create.go | 10 +++---- app/services/webhook/delete.go | 2 +- app/services/webhook/trigger.go | 4 +-- app/services/webhook/update.go | 7 +++-- .../0090_alter_webhooks_add_type.down.sql | 6 ++++ .../0090_alter_webhooks_add_type.up.sql | 6 ++++ .../0090_alter_webhooks_add_type.down.sql | 6 ++++ .../0090_alter_webhooks_add_type.up.sql | 6 ++++ app/store/database/webhook.go | 30 +++++++++---------- types/enum/webhook.go | 22 ++++++++++++++ types/webhook.go | 2 +- 15 files changed, 86 insertions(+), 39 deletions(-) create mode 100644 app/store/database/migrate/postgres/0090_alter_webhooks_add_type.down.sql create mode 100644 app/store/database/migrate/postgres/0090_alter_webhooks_add_type.up.sql create mode 100644 app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.down.sql create mode 100644 app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.up.sql diff --git a/app/api/controller/webhook/preprocessor.go b/app/api/controller/webhook/preprocessor.go index 69f29196c..534f00239 100644 --- a/app/api/controller/webhook/preprocessor.go +++ b/app/api/controller/webhook/preprocessor.go @@ -20,8 +20,8 @@ import ( ) type Preprocessor interface { - PreprocessCreateInput(enum.PrincipalType, *types.WebhookCreateInput) (bool, error) - PreprocessUpdateInput(enum.PrincipalType, *types.WebhookUpdateInput) (bool, error) + PreprocessCreateInput(enum.PrincipalType, *types.WebhookCreateInput) (enum.WebhookType, error) + PreprocessUpdateInput(enum.PrincipalType, *types.WebhookUpdateInput) (enum.WebhookType, error) PreprocessFilter(enum.PrincipalType, *types.WebhookFilter) IsInternalCall(enum.PrincipalType) bool } @@ -33,16 +33,16 @@ type NoopPreprocessor struct { func (p NoopPreprocessor) PreprocessCreateInput( enum.PrincipalType, *types.WebhookCreateInput, -) (bool, error) { - return false, nil +) (enum.WebhookType, error) { + return enum.WebhookTypeExternal, nil } // PreprocessUpdateInput always return false for internal. func (p NoopPreprocessor) PreprocessUpdateInput( enum.PrincipalType, *types.WebhookUpdateInput, -) (bool, error) { - return false, nil +) (enum.WebhookType, error) { + return enum.WebhookTypeExternal, nil } func (p NoopPreprocessor) PreprocessFilter(_ enum.PrincipalType, filter *types.WebhookFilter) { diff --git a/app/api/controller/webhook/repo_create.go b/app/api/controller/webhook/repo_create.go index e893f08db..1631dcbd5 100644 --- a/app/api/controller/webhook/repo_create.go +++ b/app/api/controller/webhook/repo_create.go @@ -35,13 +35,13 @@ func (c *Controller) CreateRepo( return nil, fmt.Errorf("failed to acquire access to the repo: %w", err) } - internal, err := c.preprocessor.PreprocessCreateInput(session.Principal.Type, in) + typ, err := c.preprocessor.PreprocessCreateInput(session.Principal.Type, in) if err != nil { return nil, fmt.Errorf("failed to preprocess create input: %w", err) } hook, err := c.webhookService.Create( - ctx, session.Principal.ID, repo.ID, enum.WebhookParentRepo, internal, in, + ctx, session.Principal.ID, repo.ID, enum.WebhookParentRepo, typ, in, ) if err != nil { return nil, fmt.Errorf("failed create webhook: %w", err) diff --git a/app/api/controller/webhook/repo_update.go b/app/api/controller/webhook/repo_update.go index b2da089f7..c7f3fdd9f 100644 --- a/app/api/controller/webhook/repo_update.go +++ b/app/api/controller/webhook/repo_update.go @@ -36,12 +36,12 @@ func (c *Controller) UpdateRepo( return nil, fmt.Errorf("failed to acquire access to the repo: %w", err) } - allowModifyingInternal, err := c.preprocessor.PreprocessUpdateInput(session.Principal.Type, in) + typ, err := c.preprocessor.PreprocessUpdateInput(session.Principal.Type, in) if err != nil { return nil, fmt.Errorf("failed to preprocess update input: %w", err) } return c.webhookService.Update( - ctx, repo.ID, enum.WebhookParentRepo, webhookIdentifier, allowModifyingInternal, in, + ctx, repo.ID, enum.WebhookParentRepo, webhookIdentifier, typ, in, ) } diff --git a/app/api/controller/webhook/space_update.go b/app/api/controller/webhook/space_update.go index cb4205a31..0af3ba733 100644 --- a/app/api/controller/webhook/space_update.go +++ b/app/api/controller/webhook/space_update.go @@ -36,12 +36,12 @@ func (c *Controller) UpdateSpace( return nil, fmt.Errorf("failed to acquire access to space: %w", err) } - allowModifyingInternal, err := c.preprocessor.PreprocessUpdateInput(session.Principal.Type, in) + typ, err := c.preprocessor.PreprocessUpdateInput(session.Principal.Type, in) if err != nil { return nil, fmt.Errorf("failed to preprocess update input: %w", err) } return c.webhookService.Update( - ctx, space.ID, enum.WebhookParentSpace, webhookIdentifier, allowModifyingInternal, in, + ctx, space.ID, enum.WebhookParentSpace, webhookIdentifier, typ, in, ) } diff --git a/app/services/webhook/create.go b/app/services/webhook/create.go index 4661b4d8c..764465a3f 100644 --- a/app/services/webhook/create.go +++ b/app/services/webhook/create.go @@ -77,10 +77,10 @@ func (s *Service) Create( principalID int64, parentID int64, parentType enum.WebhookParent, - internal bool, + typ enum.WebhookType, in *types.WebhookCreateInput, ) (*types.Webhook, error) { - err := s.sanitizeCreateInput(in, internal) + err := s.sanitizeCreateInput(in, typ == enum.WebhookTypeInternal) if err != nil { return nil, err } @@ -108,7 +108,7 @@ func (s *Service) Create( Updated: now, ParentID: parentID, ParentType: parentType, - Internal: internal, + Type: typ, Scope: scope, // user input @@ -127,7 +127,7 @@ func (s *Service) Create( // internal hooks are hidden from non-internal read requests - properly communicate their existence on duplicate. // This is best effort, any error we just ignore and fallback to original duplicate error. - if errors.Is(err, store.ErrDuplicate) && !internal { + if errors.Is(err, store.ErrDuplicate) && !(typ == enum.WebhookTypeInternal) { existingHook, derr := s.webhookStore.FindByIdentifier( ctx, enum.WebhookParentRepo, parentID, hook.Identifier) if derr != nil { @@ -137,7 +137,7 @@ func (s *Service) Create( hook.Identifier, ) } - if derr == nil && existingHook.Internal { + if derr == nil && existingHook.Type == enum.WebhookTypeInternal { return nil, errors.Conflict("The provided identifier is reserved for internal purposes.") } } diff --git a/app/services/webhook/delete.go b/app/services/webhook/delete.go index 228a4a27b..1c20b5929 100644 --- a/app/services/webhook/delete.go +++ b/app/services/webhook/delete.go @@ -33,7 +33,7 @@ func (s *Service) Delete( return err } - if hook.Internal && !allowDeletingInternal { + if hook.Type == enum.WebhookTypeInternal && !allowDeletingInternal { return ErrInternalWebhookOperationNotAllowed } diff --git a/app/services/webhook/trigger.go b/app/services/webhook/trigger.go index eb9c196b7..efdc69ea1 100644 --- a/app/services/webhook/trigger.go +++ b/app/services/webhook/trigger.go @@ -232,9 +232,9 @@ func (s *Service) executeWebhook(ctx context.Context, webhook *types.Webhook, tr // Execute HTTP Request (insecure if requested) var resp *http.Response switch { - case webhook.Internal && webhook.Insecure: + case webhook.Type == enum.WebhookTypeInternal && webhook.Insecure: resp, err = s.insecureHTTPClientInternal.Do(req) - case webhook.Internal: + case webhook.Type == enum.WebhookTypeInternal: resp, err = s.secureHTTPClientInternal.Do(req) case webhook.Insecure: resp, err = s.insecureHTTPClient.Do(req) diff --git a/app/services/webhook/update.go b/app/services/webhook/update.go index 8f53e8707..3b7f9f456 100644 --- a/app/services/webhook/update.go +++ b/app/services/webhook/update.go @@ -16,6 +16,7 @@ package webhook import ( "context" + "errors" "fmt" "github.com/harness/gitness/types" @@ -69,7 +70,7 @@ func (s *Service) Update( parentID int64, parentType enum.WebhookParent, webhookIdentifier string, - allowModifyingInternal bool, + typ enum.WebhookType, in *types.WebhookUpdateInput, ) (*types.Webhook, error) { hook, err := s.GetWebhookVerifyOwnership(ctx, parentID, parentType, webhookIdentifier) @@ -81,8 +82,8 @@ func (s *Service) Update( return nil, err } - if !allowModifyingInternal && hook.Internal { - return nil, ErrInternalWebhookOperationNotAllowed + if typ != hook.Type { + return nil, errors.New("changing type is not allowed") } // update webhook struct (only for values that are provided) diff --git a/app/store/database/migrate/postgres/0090_alter_webhooks_add_type.down.sql b/app/store/database/migrate/postgres/0090_alter_webhooks_add_type.down.sql new file mode 100644 index 000000000..b0d3c817f --- /dev/null +++ b/app/store/database/migrate/postgres/0090_alter_webhooks_add_type.down.sql @@ -0,0 +1,6 @@ +ALTER TABLE webhooks + ADD COLUMN webhook_internal BOOLEAN DEFAULT FALSE; + +UPDATE webhooks SET webhook_internal = TRUE WHERE webhook_type = 1; + +ALTER TABLE webhooks DROP COLUMN webhook_type; diff --git a/app/store/database/migrate/postgres/0090_alter_webhooks_add_type.up.sql b/app/store/database/migrate/postgres/0090_alter_webhooks_add_type.up.sql new file mode 100644 index 000000000..10f96f530 --- /dev/null +++ b/app/store/database/migrate/postgres/0090_alter_webhooks_add_type.up.sql @@ -0,0 +1,6 @@ +ALTER TABLE webhooks + ADD COLUMN webhook_type INTEGER DEFAULT 0; + +UPDATE webhooks SET webhook_type = 1 WHERE webhook_internal = TRUE; + +ALTER TABLE webhooks DROP COLUMN webhook_internal; diff --git a/app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.down.sql b/app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.down.sql new file mode 100644 index 000000000..b0d3c817f --- /dev/null +++ b/app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.down.sql @@ -0,0 +1,6 @@ +ALTER TABLE webhooks + ADD COLUMN webhook_internal BOOLEAN DEFAULT FALSE; + +UPDATE webhooks SET webhook_internal = TRUE WHERE webhook_type = 1; + +ALTER TABLE webhooks DROP COLUMN webhook_type; diff --git a/app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.up.sql b/app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.up.sql new file mode 100644 index 000000000..10f96f530 --- /dev/null +++ b/app/store/database/migrate/sqlite/0090_alter_webhooks_add_type.up.sql @@ -0,0 +1,6 @@ +ALTER TABLE webhooks + ADD COLUMN webhook_type INTEGER DEFAULT 0; + +UPDATE webhooks SET webhook_type = 1 WHERE webhook_internal = TRUE; + +ALTER TABLE webhooks DROP COLUMN webhook_internal; diff --git a/app/store/database/webhook.go b/app/store/database/webhook.go index 4fd9bf3cd..e97d165f5 100644 --- a/app/store/database/webhook.go +++ b/app/store/database/webhook.go @@ -49,15 +49,15 @@ type WebhookStore struct { // webhook is an internal representation used to store webhook data in the database. type webhook struct { - ID int64 `db:"webhook_id"` - Version int64 `db:"webhook_version"` - RepoID null.Int `db:"webhook_repo_id"` - SpaceID null.Int `db:"webhook_space_id"` - CreatedBy int64 `db:"webhook_created_by"` - Created int64 `db:"webhook_created"` - Updated int64 `db:"webhook_updated"` - Internal bool `db:"webhook_internal"` - Scope int64 `db:"webhook_scope"` + ID int64 `db:"webhook_id"` + Version int64 `db:"webhook_version"` + RepoID null.Int `db:"webhook_repo_id"` + SpaceID null.Int `db:"webhook_space_id"` + CreatedBy int64 `db:"webhook_created_by"` + Created int64 `db:"webhook_created"` + Updated int64 `db:"webhook_updated"` + Type enum.WebhookType `db:"webhook_type"` + Scope int64 `db:"webhook_scope"` Identifier string `db:"webhook_uid"` // TODO [CODE-1364]: Remove once UID/Identifier migration is completed. @@ -89,7 +89,7 @@ const ( ,webhook_insecure ,webhook_triggers ,webhook_latest_execution_result - ,webhook_internal + ,webhook_type ,webhook_scope` webhookSelectBase = ` @@ -176,7 +176,7 @@ func (s *WebhookStore) Create(ctx context.Context, hook *types.Webhook) error { ,webhook_insecure ,webhook_triggers ,webhook_latest_execution_result - ,webhook_internal + ,webhook_type ,webhook_scope ) values ( :webhook_repo_id @@ -193,7 +193,7 @@ func (s *WebhookStore) Create(ctx context.Context, hook *types.Webhook) error { ,:webhook_insecure ,:webhook_triggers ,:webhook_latest_execution_result - ,:webhook_internal + ,:webhook_type ,:webhook_scope ) RETURNING webhook_id` @@ -457,7 +457,7 @@ func mapToWebhook(hook *webhook) (*types.Webhook, error) { Insecure: hook.Insecure, Triggers: triggersFromString(hook.Triggers), LatestExecutionResult: (*enum.WebhookExecutionResult)(hook.LatestExecutionResult.Ptr()), - Internal: hook.Internal, + Type: hook.Type, } switch { @@ -494,7 +494,7 @@ func mapToInternalWebhook(hook *types.Webhook) (*webhook, error) { Insecure: hook.Insecure, Triggers: triggersToString(hook.Triggers), LatestExecutionResult: null.StringFromPtr((*string)(hook.LatestExecutionResult)), - Internal: hook.Internal, + Type: hook.Type, } switch hook.ParentType { @@ -559,7 +559,7 @@ func applyWebhookFilter( } if opts.SkipInternal { - stmt = stmt.Where("webhook_internal != ?", true) + stmt = stmt.Where("webhook_type == ?", enum.WebhookTypeExternal) } return stmt diff --git a/types/enum/webhook.go b/types/enum/webhook.go index d52c6c434..430b48c50 100644 --- a/types/enum/webhook.go +++ b/types/enum/webhook.go @@ -121,6 +121,28 @@ var webhookExecutionResults = sortEnum([]WebhookExecutionResult{ WebhookExecutionResultFatalError, }) +// WebhookType defines different types of a webhook. +type WebhookType int + +func (WebhookType) Enum() []interface{} { return toInterfaceSlice(webhookTypes) } + +const ( + // WebhookTypeExternal describes a webhook url pointing to external source. + WebhookTypeExternal WebhookType = iota + + // WebhookTypeInternal describes a webhook url pointing to internal url. + WebhookTypeInternal + + // WebhookTypeJira describes a webhook url pointing to jira. + WebhookTypeJira +) + +var webhookTypes = sortEnum([]WebhookType{ + WebhookTypeExternal, + WebhookTypeInternal, + WebhookTypeJira, +}) + // WebhookTrigger defines the different types of webhook triggers available. type WebhookTrigger string diff --git a/types/webhook.go b/types/webhook.go index 93a8a6072..e701a0fc4 100644 --- a/types/webhook.go +++ b/types/webhook.go @@ -30,7 +30,7 @@ type Webhook struct { CreatedBy int64 `json:"created_by"` Created int64 `json:"created"` Updated int64 `json:"updated"` - Internal bool `json:"-"` + Type enum.WebhookType `json:"-"` // scope 0 indicates repo; scope > 0 indicates space depth level Scope int64 `json:"scope"`