From 599d12a04dec504b0a407734442b98d39ac87793 Mon Sep 17 00:00:00 2001 From: peimingming Date: Wed, 21 Aug 2019 14:57:09 +0800 Subject: [PATCH] Fix bugs by comments for webhook Signed-off-by: peimingming --- src/chartserver/reverse_proxy.go | 18 ++++++------ src/core/api/chart_repository.go | 28 ++++++++++--------- src/core/api/repository.go | 11 ++++---- .../handler/notification/processor.go | 19 ++++++++----- .../notification/scan_image_handler.go | 3 ++ .../notification/scan_image_handler_test.go | 2 +- .../service/notifications/jobs/handler.go | 9 +++--- .../service/notifications/registry/handler.go | 22 ++++++++------- 8 files changed, 64 insertions(+), 48 deletions(-) diff --git a/src/chartserver/reverse_proxy.go b/src/chartserver/reverse_proxy.go index b0804d662..30da495f2 100644 --- a/src/chartserver/reverse_proxy.go +++ b/src/chartserver/reverse_proxy.go @@ -126,12 +126,13 @@ func modifyResponse(res *http.Response) error { Operator: e.Resource.ExtendedInfo["operator"].(string), }, } - if err := event.Build(metaData); err != nil { + if err := event.Build(metaData); err == nil { + if err := event.Publish(); err != nil { + hlog.Errorf("failed to publish chart upload event: %v", err) + } + } else { hlog.Errorf("failed to build chart upload event metadata: %v", err) } - if err := event.Publish(); err != nil { - hlog.Errorf("failed to publish chart upload event: %v", err) - } } } } @@ -143,12 +144,13 @@ func modifyResponse(res *http.Response) error { if ok && eventMetaData != nil { // Trigger harbor webhook event := &n_event.Event{} - if err := event.Build(eventMetaData); err != nil { + if err := event.Build(eventMetaData); err == nil { + if err := event.Publish(); err != nil { + hlog.Errorf("failed to publish chart download event: %v", err) + } + } else { hlog.Errorf("failed to build chart download event metadata: %v", err) } - if err := event.Publish(); err != nil { - hlog.Errorf("failed to publish chart download event: %v", err) - } } } diff --git a/src/core/api/chart_repository.go b/src/core/api/chart_repository.go index 9e4f16d9c..5a67acdb8 100755 --- a/src/core/api/chart_repository.go +++ b/src/core/api/chart_repository.go @@ -301,12 +301,13 @@ func (cra *ChartRepositoryAPI) DeleteChartVersion() { Operator: cra.SecurityCtx.GetUsername(), }, } - if err := event.Build(metaData); err != nil { + if err := event.Build(metaData); err == nil { + if err := event.Publish(); err != nil { + hlog.Errorf("failed to publish chart delete event: %v", err) + } + } else { hlog.Errorf("failed to build chart delete event metadata: %v", err) } - if err := event.Publish(); err != nil { - hlog.Errorf("failed to publish chart delete event: %v", err) - } } // UploadChartVersion handles POST /api/:repo/charts @@ -393,6 +394,11 @@ func (cra *ChartRepositoryAPI) DeleteChart() { } } + if err := chartController.DeleteChart(cra.namespace, chartName); err != nil { + cra.SendInternalServerError(err) + return + } + event := &n_event.Event{} metaData := &n_event.ChartDeleteMetaData{ ChartMetaData: n_event.ChartMetaData{ @@ -403,17 +409,13 @@ func (cra *ChartRepositoryAPI) DeleteChart() { Operator: cra.SecurityCtx.GetUsername(), }, } - if err := event.Build(metaData); err != nil { + if err := event.Build(metaData); err == nil { + if err := event.Publish(); err != nil { + hlog.Errorf("failed to publish chart delete event: %v", err) + } + } else { hlog.Errorf("failed to build chart delete event metadata: %v", err) } - if err := event.Publish(); err != nil { - hlog.Errorf("failed to publish chart delete event: %v", err) - } - - if err := chartController.DeleteChart(cra.namespace, chartName); err != nil { - cra.SendInternalServerError(err) - return - } } func (cra *ChartRepositoryAPI) removeLabelsFromChart(chartName, version string) error { diff --git a/src/core/api/repository.go b/src/core/api/repository.go index 5da092b10..bea194509 100755 --- a/src/core/api/repository.go +++ b/src/core/api/repository.go @@ -349,14 +349,15 @@ func (ra *RepositoryAPI) Delete() { OccurAt: time.Now(), Operator: ra.SecurityCtx.GetUsername(), } - if err := evt.Build(imgDelMetadata); err != nil { + if err := evt.Build(imgDelMetadata); err == nil { + if err := evt.Publish(); err != nil { + // do not return when publishing event failed + log.Errorf("failed to publish image delete event: %v", err) + } + } else { // do not return when building event metadata failed log.Errorf("failed to build image delete event metadata: %v", err) } - if err := evt.Publish(); err != nil { - // do not return when publishing event failed - log.Errorf("failed to publish image delete event: %v", err) - } exist, err := repositoryExist(repoName, rc) if err != nil { diff --git a/src/core/notifier/handler/notification/processor.go b/src/core/notifier/handler/notification/processor.go index a2b3f5524..8f9163c49 100644 --- a/src/core/notifier/handler/notification/processor.go +++ b/src/core/notifier/handler/notification/processor.go @@ -98,6 +98,7 @@ func constructImagePayload(event *notifyModel.ImageEvent) (*notifyModel.Payload, // send hook by publishing topic of specified target type(notify type) func sendHookWithPolicies(policies []*models.NotificationPolicy, payload *notifyModel.Payload, eventType string) error { + errRet := false for _, ply := range policies { targets := ply.Targets for _, target := range targets { @@ -108,18 +109,22 @@ func sendHookWithPolicies(policies []*models.NotificationPolicy, payload *notify Payload: payload, Target: &target, } - if err := evt.Build(hookMetadata); err != nil { + // It should never affect evaluating other policies when one is failed, but error should return + if err := evt.Build(hookMetadata); err == nil { + if err := evt.Publish(); err != nil { + errRet = true + log.Errorf("failed to publish hook notify event: %v", err) + } + } else { + errRet = true log.Errorf("failed to build hook notify event metadata: %v", err) - return err } - if err := evt.Publish(); err != nil { - log.Errorf("failed to publish hook notify event: %v", err) - return err - } - log.Debugf("published image event %s by topic %s", payload.Type, target.Type) } } + if errRet { + return errors.New("failed to trigger some of the events") + } return nil } diff --git a/src/core/notifier/handler/notification/scan_image_handler.go b/src/core/notifier/handler/notification/scan_image_handler.go index c3e479e8e..1e972e972 100644 --- a/src/core/notifier/handler/notification/scan_image_handler.go +++ b/src/core/notifier/handler/notification/scan_image_handler.go @@ -52,6 +52,9 @@ func (si *ScanImagePreprocessHandler) Handle(value interface{}) error { log.Errorf("failed to find project[%s] for scan image event: %v", projectName, err) return err } + if project == nil { + return fmt.Errorf("project[%s] not found", projectName) + } policies, err := notification.PolicyMgr.GetRelatedPolices(project.ProjectID, e.EventType) if err != nil { log.Errorf("failed to find policy for %s event: %v", e.EventType, err) diff --git a/src/core/notifier/handler/notification/scan_image_handler_test.go b/src/core/notifier/handler/notification/scan_image_handler_test.go index 9eee2b154..163762c53 100644 --- a/src/core/notifier/handler/notification/scan_image_handler_test.go +++ b/src/core/notifier/handler/notification/scan_image_handler_test.go @@ -94,7 +94,7 @@ func TestScanImagePreprocessHandler_Handle(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := handler.Handle(tt.args.data) if tt.wantErr { - require.NotNil(t, err, "Error: %s", err) + require.NotNil(t, err, "Error: %v", err) return } assert.Nil(t, err) diff --git a/src/core/service/notifications/jobs/handler.go b/src/core/service/notifications/jobs/handler.go index dbf6d5f11..30361eb43 100755 --- a/src/core/service/notifications/jobs/handler.go +++ b/src/core/service/notifications/jobs/handler.go @@ -89,12 +89,13 @@ func (h *Handler) HandleScan() { JobID: h.id, Status: h.status, } - if err := e.Build(metaData); err != nil { + if err := e.Build(metaData); err == nil { + if err := e.Publish(); err != nil { + log.Errorf("failed to publish image scanning event: %v", err) + } + } else { log.Errorf("failed to build image scanning event metadata: %v", err) } - if err := e.Publish(); err != nil { - log.Errorf("failed to publish image scanning event: %v", err) - } } if err := dao.UpdateScanJobStatus(h.id, h.status); err != nil { diff --git a/src/core/service/notifications/registry/handler.go b/src/core/service/notifications/registry/handler.go index 351938a45..3887001ad 100755 --- a/src/core/service/notifications/registry/handler.go +++ b/src/core/service/notifications/registry/handler.go @@ -127,14 +127,15 @@ func (n *NotificationHandler) Post() { OccurAt: time.Now(), Operator: event.Actor.Name, } - if err := evt.Build(imgPushMetadata); err != nil { + if err := evt.Build(imgPushMetadata); err == nil { + if err := evt.Publish(); err != nil { + // do not return when publishing event failed + log.Errorf("failed to publish image push event: %v", err) + } + } else { // do not return when building event metadata failed log.Errorf("failed to build image push event metadata: %v", err) } - if err := evt.Publish(); err != nil { - // do not return when publishing event failed - log.Errorf("failed to publish image push event: %v", err) - } // TODO: handle image delete event and chart event go func() { @@ -178,14 +179,15 @@ func (n *NotificationHandler) Post() { OccurAt: time.Now(), Operator: event.Actor.Name, } - if err := evt.Build(imgPullMetadata); err != nil { + if err := evt.Build(imgPullMetadata); err == nil { + if err := evt.Publish(); err != nil { + // do not return when publishing event failed + log.Errorf("failed to publish image pull event: %v", err) + } + } else { // do not return when building event metadata failed log.Errorf("failed to build image push event metadata: %v", err) } - if err := evt.Publish(); err != nil { - // do not return when publishing event failed - log.Errorf("failed to publish image pull event: %v", err) - } go func() { log.Debugf("Increase the repository %s pull count.", repository)