fix: Process webhook refresh in background to not block the request (a…

…rgoproj#14269)

Signed-off-by: dhruvang1 <[email protected]>

applicationset/webhook/webhook.go

@@ -5,15 +5,15 @@ import (
  "errors"
  "fmt"
  "html"
+ "k8s.io/apimachinery/pkg/types"
+ "k8s.io/client-go/util/retry"
  "net/http"
  "net/url"
  "regexp"
+ "sigs.k8s.io/controller-runtime/pkg/client"
  "strconv"
  "strings"
-
- "k8s.io/apimachinery/pkg/types"
- "k8s.io/client-go/util/retry"
- "sigs.k8s.io/controller-runtime/pkg/client"
+ "sync"
 
  "github.com/argoproj/argo-cd/v2/applicationset/generators"
  "github.com/argoproj/argo-cd/v2/common"
@@ -31,6 +31,7 @@ var (
 )
 
 type WebhookHandler struct {
+ sync.WaitGroup // for testing
  namespace string
  github *github.Webhook
  gitlab *gitlab.Webhook
@@ -178,7 +179,11 @@ func (h *WebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
  return
  }
 
- h.HandleEvent(payload)
+ h.Add(1)
+ go func() {
+ defer h.Done()
+ h.HandleEvent(payload)
+ }()
 }
 
 func parseRevision(ref string) string {

applicationset/webhook/webhook_test.go

@@ -215,6 +215,7 @@ func TestWebhookHandler(t *testing.T) {
  w := httptest.NewRecorder()
 
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, test.expectedStatusCode)
 
  list := &v1alpha1.ApplicationSetList{}

util/webhook/webhook.go

@@ -9,6 +9,7 @@ import (
  "net/url"
  "regexp"
  "strings"
+ "sync"
 
  "github.com/go-playground/webhooks/v6/azuredevops"
  "github.com/go-playground/webhooks/v6/bitbucket"
@@ -47,6 +48,7 @@ var (
 )
 
 type ArgoCDWebhookHandler struct {
+ sync.WaitGroup // for testing
  repoCache *cache.Cache
  serverCache *servercache.Cache
  db db.ArgoDB
@@ -439,5 +441,9 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
  return
  }
 
- a.HandleEvent(payload)
+ a.Add(1)
+ go func() {
+ defer a.Done()
+ a.HandleEvent(payload)
+ }()
 }

util/webhook/webhook_test.go

@@ -85,6 +85,7 @@ func TestGitHubCommitEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Received push event repo: https://github.com/jessesuen/test-repo, revision: master, touchedHead: true"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -101,6 +102,7 @@ func TestAzureDevOpsCommitEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Received push event repo: https://dev.azure.com/alexander0053/alex-test/_git/alex-test, revision: master, touchedHead: true"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -156,6 +158,7 @@ func TestGitHubCommitEvent_MultiSource_Refresh(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Requested app 'app-to-refresh' refresh"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -237,6 +240,7 @@ func TestGitHubCommitEvent_AppsInOtherNamespaces(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
 
  logMessages := make([]string, 0, len(hook.Entries))
@@ -269,6 +273,7 @@ func TestGitHubTagEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Received push event repo: https://github.com/jessesuen/test-repo, revision: v1.0, touchedHead: false"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -285,6 +290,7 @@ func TestGitHubPingEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Ignoring webhook event"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -301,6 +307,7 @@ func TestBitbucketServerRepositoryReferenceChangedEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResultSsh := "Received push event repo: ssh://git@bitbucketserver:7999/myproject/test-repo.git, revision: master, touchedHead: true"
  assert.Equal(t, expectedLogResultSsh, hook.AllEntries()[len(hook.AllEntries())-2].Message)
@@ -317,6 +324,7 @@ func TestBitbucketServerRepositoryDiagnosticPingEvent(t *testing.T) {
  req.Header.Set("X-Event-Key", "diagnostics:ping")
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Ignoring webhook event"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -333,6 +341,7 @@ func TestGogsPushEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Received push event repo: http://gogs-server/john/repo-test, revision: master, touchedHead: true"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -349,6 +358,7 @@ func TestGitLabPushEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Received push event repo: https://gitlab/group/name, revision: master, touchedHead: true"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -365,6 +375,7 @@ func TestGitLabSystemEvent(t *testing.T) {
  req.Body = io.NopCloser(bytes.NewReader(eventJSON))
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusOK)
  expectedLogResult := "Received push event repo: https://gitlab/group/name, revision: master, touchedHead: true"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -378,6 +389,7 @@ func TestInvalidMethod(t *testing.T) {
  req.Header.Set("X-GitHub-Event", "push")
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusMethodNotAllowed)
  expectedLogResult := "Webhook processing failed: invalid HTTP Method"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -392,6 +404,7 @@ func TestInvalidEvent(t *testing.T) {
  req.Header.Set("X-GitHub-Event", "push")
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusBadRequest)
  expectedLogResult := "Webhook processing failed: error parsing payload"
  assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
@@ -406,6 +419,7 @@ func TestUnknownEvent(t *testing.T) {
  req.Header.Set("X-Unknown-Event", "push")
  w := httptest.NewRecorder()
  h.Handler(w, req)
+ h.Wait()
  assert.Equal(t, w.Code, http.StatusBadRequest)
  assert.Equal(t, "Unknown webhook event\n", w.Body.String())
  hook.Reset()