add ability to send webhooks to external tools #367

alessandro-sorint · 2023-01-16T11:26:30Z

No description provided.

internal/services/config/config.go

internal/services/notification/runstatus.go

services/runservice/types/runevent.go

internal/services/notification/runevents.go

tests/setup_test.go

internal/services/config/config.go

internal/services/notification/runstatus.go

internal/services/notification/runevents.go

tests/webhooks_receiver.go

sgotti · 2023-02-03T12:40:31Z

tests/webhooks_receiver.go

+func NewRunEventsHandler(log zerolog.Logger, runPhases *[]rstypes.RunPhase) *runEventsHandler {
+	return &runEventsHandler{log: log, runPhases: runPhases}
+}
+
+func (h *runEventsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if err := util.HTTPResponse(w, http.StatusOK, h.runPhases); err != nil {
+		h.log.Err(err).Send()
+	}
+}


This isn't needed

tests/webhooks_receiver.go

internal/services/notification/runstatus.go

tests/webhooksreceiver.go

internal/services/notification/runevents.go

tests/webhooksreceiver.go

sgotti · 2023-06-22T11:20:47Z

internal/migration/destination/runservice/db/ddl.go

internal/migration should not be changed.

sgotti · 2023-06-26T13:23:13Z

internal/services/notification/runevents.go

@@ -92,7 +92,7 @@ func (n *NotificationService) runEventsHandler(ctx context.Context) error {
 			data := buf.Bytes()
 			buf.Reset()

-			var ev *rstypes.RunEvent


keep the pointer as before var ev *rstypes.RunEvent

sgotti · 2023-06-26T13:31:15Z

services/runservice/types/runevent.go

+	Annotations map[string]string         `json:"annotations,omitempty"`
+}
+
+type RunStatusTask struct {


RunEventDataRunTask

sgotti · 2023-06-26T13:31:54Z

services/runservice/types/runevent.go

+	Name            string                          `json:"name,omitempty"`
+	Level           int                             `json:"level,omitempty"`
+	Skip            bool                            `json:"skip,omitempty"`
+	Depends         map[string]*RunStatusTaskDepend `json:"depends,omitempty"`


RunEventDataRunTaskDepend

internal/services/notification/webhooks.go

alessandro-sorint · 2023-06-28T07:33:30Z

@sgotti can you do the review please?

sgotti · 2023-06-28T07:55:05Z

internal/services/notification/runevents.go

+					}
+				}
+			default:
+				n.log.Info().Msgf("run event %q is not valid", ev.RunEventType)


Error level ?

sgotti · 2023-06-28T07:55:16Z

internal/services/notification/runevents.go

+				}
+				if n.c.WebhookURL != "" {
+					if err := n.sendWebhooks(ctx, ev); err != nil {
+						n.log.Info().Msgf("failed to update run status")


Error level?

sgotti · 2023-06-28T07:55:36Z

internal/services/notification/runevents.go

+			switch ev.RunEventType {
+			case rstypes.RunPhaseChanged:
+				if err := n.updateCommitStatus(ctx, ev); err != nil {
+					n.log.Info().Msgf("failed to update commit status")


Error level?

sgotti · 2023-06-28T07:56:36Z

internal/services/notification/webhooks.go

+
+	// the trigger of the event
+	// (i.e. the value run_phase_changed indicate a run phase changed event)
+	EventType string `json:"event_type"`


What is EventType? Is it the same as github webhook action field? If so call it action since we already have the Event header and the name are too similar and confusing.

See https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads#pull_request for further info.

sgotti · 2023-06-28T07:57:28Z

internal/services/notification/webhooks.go

+}
+
+type ProjectInfo struct {
+	// id of the project


always start descriptions with the variable name:

// ProjectID is the ID of the project

But in this case I don't think it's needed.

sgotti · 2023-06-28T08:06:14Z

internal/services/notification/webhooks.go

+)
+
+type RunWebhook struct {
+	// version of the webhook struct data


always start descriptions with the variable name

sgotti · 2023-06-28T08:07:05Z

internal/services/notification/webhooks.go

+
+	// the trigger of the event
+	// (i.e. the value run_phase_changed indicate a run phase changed event)
+	EventType string `json:"event_type"`


Use a dedicated enum for EventType/Action and add the description there and not here

sgotti · 2023-06-28T08:07:43Z

internal/services/notification/webhooks.go

+		return errors.WithStack(err)
+	}
+	req.Header.Add("Content-Type", "application/json")
+	req.Header.Add(agolaEventHeader, "run")


Use a dedicated enum for the event header values

services/runservice/types/runevent.go

alessandro-sorint · 2023-06-29T09:18:15Z

@sgotti can you do the review please?

sgotti · 2023-06-29T11:21:48Z

services/runservice/types/runevent.go

+
+func (e *RunEvent) PreJSON() error {
+	switch e.DataVersion {
+


not needed empty line

internal/services/notification/webhooks.go

notification: add ability to send webhook on run events config: add configuration for webhooks

sgotti

@alessandro-sorint We can go with this, given that this is just a first step and the data format won't be stable until 0.9.0. I also have different changes that I'll do in future.

alessandro-sorint force-pushed the webhook-notification branch 3 times, most recently from c3d420f to ca16706 Compare January 30, 2023 09:23

alessandro-sorint force-pushed the webhook-notification branch from ca16706 to 1bb65c9 Compare February 1, 2023 11:15

sgotti requested changes Feb 2, 2023

View reviewed changes

internal/services/config/config.go Outdated Show resolved Hide resolved

internal/services/config/config.go Outdated Show resolved Hide resolved

sgotti reviewed Feb 2, 2023

View reviewed changes

internal/services/notification/runstatus.go Outdated Show resolved Hide resolved

alessandro-sorint force-pushed the webhook-notification branch 2 times, most recently from 210e39f to c517311 Compare February 2, 2023 15:56

alessandro-sorint requested a review from sgotti February 2, 2023 16:14

sgotti requested changes Feb 2, 2023

View reviewed changes

internal/services/notification/runstatus.go Outdated Show resolved Hide resolved

internal/services/notification/runstatus.go Outdated Show resolved Hide resolved

alessandro-sorint force-pushed the webhook-notification branch 3 times, most recently from c050e03 to 6997687 Compare February 3, 2023 11:24

sgotti requested changes Feb 3, 2023

View reviewed changes

alessandro-sorint force-pushed the webhook-notification branch from 6997687 to 91f09ea Compare February 6, 2023 08:13

sgotti requested changes Feb 6, 2023

View reviewed changes

internal/services/notification/runstatus.go Outdated Show resolved Hide resolved

tests/webhooksreceiver.go Outdated Show resolved Hide resolved

tests/webhooksreceiver.go Outdated Show resolved Hide resolved

alessandro-sorint force-pushed the webhook-notification branch 3 times, most recently from 586a6f3 to fb1e4bb Compare February 6, 2023 15:26

sgotti requested changes Feb 6, 2023

View reviewed changes

internal/services/notification/runevents.go Outdated Show resolved Hide resolved

tests/webhooksreceiver.go Outdated Show resolved Hide resolved

alessandro-sorint force-pushed the webhook-notification branch 2 times, most recently from d7d8274 to b47ebeb Compare February 8, 2023 10:21

alessandro-sorint requested a review from sgotti February 8, 2023 11:24

sgotti reviewed Feb 8, 2023

View reviewed changes

tests/webhooksreceiver.go Show resolved Hide resolved

alessandro-sorint force-pushed the webhook-notification branch from b47ebeb to 8434abc Compare February 8, 2023 13:46

alessandro-sorint requested a review from sgotti February 8, 2023 14:15

alessandro-sorint force-pushed the webhook-notification branch 3 times, most recently from a780086 to e9ba480 Compare May 8, 2023 10:24

alessandro-sorint force-pushed the webhook-notification branch 4 times, most recently from 19de66a to 3985e48 Compare June 22, 2023 09:54

alessandro-sorint requested a review from sgotti June 22, 2023 12:05

sgotti requested changes Jun 26, 2023

View reviewed changes

internal/migration/destination/runservice/db/ddl.go Outdated

Copy link

Member

sgotti Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal/migration should not be changed.

alessandro-sorint force-pushed the webhook-notification branch from 3985e48 to 6c76360 Compare June 26, 2023 09:10

alessandro-sorint requested a review from sgotti June 26, 2023 12:44

sgotti reviewed Jun 26, 2023

View reviewed changes

alessandro-sorint force-pushed the webhook-notification branch from 6c76360 to 0c4f1d0 Compare June 26, 2023 14:58

alessandro-sorint requested a review from sgotti June 27, 2023 07:14

sgotti requested changes Jun 28, 2023

View reviewed changes

alessandro-sorint force-pushed the webhook-notification branch 2 times, most recently from 00a0571 to 8ef7eb7 Compare June 28, 2023 13:36

alessandro-sorint requested a review from sgotti June 28, 2023 14:43

alessandro-sorint force-pushed the webhook-notification branch from 8ef7eb7 to 6e86b45 Compare June 29, 2023 12:04

sgotti requested changes Jun 29, 2023

View reviewed changes

services/runservice/types/runevent.go Outdated

func (e *RunEvent) PreJSON() error {

switch e.DataVersion {

Copy link

Member

sgotti Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed empty line

alessandro-sorint force-pushed the webhook-notification branch from 6e86b45 to 35840ca Compare June 29, 2023 12:11

alessandro-sorint requested a review from sgotti June 29, 2023 12:12

sgotti requested changes Jun 29, 2023

View reviewed changes

internal/services/notification/webhooks.go Outdated Show resolved Hide resolved

internal/services/notification/webhooks.go Outdated Show resolved Hide resolved

internal/services/notification/webhooks.go Outdated Show resolved Hide resolved

alessandro-sorint force-pushed the webhook-notification branch from 35840ca to 7526c64 Compare June 29, 2023 12:44

alessandro-sorint requested a review from sgotti June 29, 2023 12:45

alessandro-sorint force-pushed the webhook-notification branch from 7526c64 to dabda70 Compare June 29, 2023 13:18

add ability to send webhooks to external tools

cdb81d6

notification: add ability to send webhook on run events config: add configuration for webhooks

alessandro-sorint force-pushed the webhook-notification branch from dabda70 to cdb81d6 Compare June 29, 2023 13:37

sgotti approved these changes Jun 29, 2023

View reviewed changes

sgotti merged commit a61634a into agola-io:master Jun 29, 2023
1 check passed

alessandro-sorint deleted the webhook-notification branch June 30, 2023 06:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add ability to send webhooks to external tools #367

add ability to send webhooks to external tools #367

alessandro-sorint commented Jan 16, 2023

sgotti Feb 3, 2023

sgotti Jun 22, 2023

sgotti Jun 26, 2023

sgotti Jun 26, 2023

sgotti Jun 26, 2023

alessandro-sorint commented Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

sgotti Jun 28, 2023

alessandro-sorint commented Jun 29, 2023

sgotti Jun 29, 2023

sgotti left a comment

add ability to send webhooks to external tools #367

add ability to send webhooks to external tools #367

Conversation

alessandro-sorint commented Jan 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessandro-sorint commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessandro-sorint commented Jun 29, 2023

Choose a reason for hiding this comment

sgotti left a comment

Choose a reason for hiding this comment