Update status code handling

In the current version of the product, the HTTP request to the EventListener
endpoint waits for all triggers to process to determine if a resource was
created. This is not in line with the suggestion by the source control
systems for webhook endpoints. This change updates the endpoint to respond
as soon as the triggers have been selected for processing.

cmd/eventlistenersink/main.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"sync"
 	"time"
 
 	dynamicClientset "github.com/tektoncd/triggers/pkg/client/dynamic/clientset"
@@ -131,6 +132,7 @@ func main() {
 		Logger:                 logger,
 		Recorder:               recorder,
 		Auth:                   sink.DefaultAuthOverride{},
+		WaitGroup:              &sync.WaitGroup{},
 		// Register all the listers we'll need
 		EventListenerLister:         factory.Triggers().V1alpha1().EventListeners().Lister(),
 		TriggerLister:               factory.Triggers().V1alpha1().Triggers().Lister(),

cmd/triggerrun/cmd/root.go

@@ -28,6 +28,7 @@ import (
 	"log"
 	"net/http"
 	"os"
+	"sync"
 
 	"github.com/spf13/cobra"
 	"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
@@ -261,6 +262,7 @@ func newSink(config *rest.Config, sugerLogger *zap.SugaredLogger) sink.Sink {
 		KubeClientSet:          kubeClient,
 		HTTPClient:             http.DefaultClient,
 		Auth:                   sink.DefaultAuthOverride{},
+		WaitGroup:              &sync.WaitGroup{},
 		DiscoveryClient:        sinkClients.DiscoveryClient,
 		DynamicClient:          dynamicCS,
 		Logger:                 sugerLogger,

cmd/triggerrun/cmd/root_test.go

@@ -23,6 +23,7 @@ import (
 	"net/http/httputil"
 	"regexp"
 	"strings"
+	"sync"
 	"testing"
 
 	"go.uber.org/zap/zaptest"
@@ -237,6 +238,7 @@ func Test_processTriggerSpec(t *testing.T) {
 			s := sink.Sink{
 				KubeClientSet: kubeClient,
 				HTTPClient:    http.DefaultClient,
+				WaitGroup:     &sync.WaitGroup{},
 			}
 			got, err := processTriggerSpec(kubeClient, triggerClient, tt.args.t, tt.args.request, tt.args.event, eventID, logger, s)
 			if (err != nil) != tt.wantErr {

docs/eventlisteners.md

@@ -370,8 +370,9 @@ metadata:
 
 ## Understanding `EventListener` response
 
-An `EventListener` responds with a `201 CREATED` HTTP response when at least one specified `Trigger` executes successfully.
-Otherwise, it responds with a `202 ACCEPTED` HTTP response. 
+An `EventListener` responds with a `202 ACCEPTED` HTTP response when the `EventListener`
+has been able to process the request and selected the appropriate triggers to process
+based off the `EventListener` configuration. 
 
 After detecting an event, the `EventListener` responds with the following message:
 

examples/bitbucket/README.md

@@ -28,7 +28,7 @@ Creates an EventListener that listens for Bitbucket webhook events.
    http://localhost:8080
    ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
    [`HMAC`](https://www.freeformatter.com/hmac-generator.html) tool used to create X-Hub-Signature.
 

examples/custom-resource/README.md

@@ -21,7 +21,7 @@ Creates an EventListener that listens for GitHub webhook events.
    http://<el_address>
    ```
 
-   The response status code is `201 Created`
+   The response status code should be `202 Accepted`
 
    [`HMAC`](https://www.freeformatter.com/hmac-generator.html) tool used to create X-Hub-Signature.
 

examples/eventlistener-tls-connection/README.md

@@ -64,7 +64,7 @@ This request will be processed by the owner of the root key to generate the cert
    https://<el-address> --cacert rootCA.crt --key client.key --cert client.crt
    ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
    [`HMAC`](https://www.freeformatter.com/hmac-generator.html) tool used to create X-Hub-Signature.
 

examples/github/README.md

@@ -29,7 +29,7 @@ Creates an EventListener that listens for GitHub webhook events.
    http://localhost:8080
    ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
    [`HMAC`](https://www.freeformatter.com/hmac-generator.html) tool used to create X-Hub-Signature.
 

examples/gitlab/README.md

@@ -29,7 +29,7 @@ Creates an EventListener that listens for GitLab webhook events.
    http://localhost:8080
    ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
 1. You should see a new TaskRun that got created:
 

examples/selectors/label/README.md

@@ -31,7 +31,7 @@ Creates an EventListener that serve triggers selected via a label selector.
        -H 'Content-Type: application/json' \
        -d '{"head_commit":{"id":"28911bbb5a3e2ea034daf1f6be0a822d50e31e73"},"action": "opened", "pull_request":{"head":{"sha": "28911bbb5a3e2ea034daf1f6be0a822d50e31e73"}},"repository":{"clone_url": "https://github.com/tektoncd/triggers.git", "url":"https://github.com/tektoncd/triggers.git"}}' http://localhost:8000   ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
 4. You should see a single new Pipelinerun gets created, even though there are two triggers that would match the request data in the `foo` namespace
 

examples/selectors/namespace/README.md

@@ -31,7 +31,7 @@ Creates an EventListener that serve triggers in multiple namespaces.
        -H 'Content-Type: application/json' \
        -d '{"head_commit":{"id":"28911bbb5a3e2ea034daf1f6be0a822d50e31e73"},"action": "opened", "pull_request":{"head":{"sha": "28911bbb5a3e2ea034daf1f6be0a822d50e31e73"}},"repository":{"clone_url": "https://github.com/tektoncd/triggers.git", "url":"https://github.com/tektoncd/triggers.git"}}' http://localhost:8000   ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
 4. You should see a new Pipelinerun that got created:
 

examples/v1alpha1-task/README.md

@@ -27,7 +27,7 @@ Creates an EventListener that creates a v1alpha1 TaskRun.
    http://localhost:8080
    ```
 
-   The response status code should be `201 Created`
+   The response status code should be `202 Accepted`
 
 1. You should see a new TaskRun that got created:
 

pkg/sink/metrics_test.go

@@ -3,6 +3,7 @@ package sink
 import (
 	"context"
 	"encoding/json"
+	"sync"
 	"testing"
 
 	"go.opencensus.io/stats/view"
@@ -67,7 +68,8 @@ func TestRecordResourceCreation(t *testing.T) {
 			}
 			r, _ := NewRecorder()
 			s := &Sink{
-				Recorder: r,
+				Recorder:  r,
+				WaitGroup: &sync.WaitGroup{},
 			}
 			s.recordResourceCreation(test.resources)
 			rows, err := view.RetrieveData("triggered_resources")

pkg/sink/sink.go

@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"sync"
 
 	triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
 	triggersclientset "github.com/tektoncd/triggers/pkg/client/clientset/versioned"
@@ -34,7 +35,6 @@ import (
 	"github.com/tektoncd/triggers/pkg/template"
 	"github.com/tidwall/sjson"
 	"go.uber.org/zap"
-	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	discoveryclient "k8s.io/client-go/discovery"
@@ -55,6 +55,7 @@ type Sink struct {
 	Logger                 *zap.SugaredLogger
 	Recorder               *Recorder
 	Auth                   AuthOverride
+	WaitGroup              *sync.WaitGroup
 
 	// listers index properties about resources
 	EventListenerLister         listers.EventListenerLister
@@ -147,45 +148,17 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) {
 		response.WriteHeader(http.StatusInternalServerError)
 		return
 	}
-	result := make(chan int, 10)
 	// Execute each Trigger
+	r.WaitGroup.Add(len(triggers))
 	for _, t := range triggers {
 		go func(t triggersv1.Trigger) {
+			defer r.WaitGroup.Done()
 			localRequest := request.Clone(request.Context())
-			if err := r.processTrigger(t, localRequest, event, eventID, eventLog); err != nil {
-				if kerrors.IsUnauthorized(err) {
-					result <- http.StatusUnauthorized
-					return
-				}
-				if kerrors.IsForbidden(err) {
-					result <- http.StatusForbidden
-					return
-				}
-				result <- http.StatusAccepted
-				return
-			}
-			result <- http.StatusCreated
+			r.processTrigger(t, localRequest, event, eventID, eventLog)
 		}(*t)
 	}
 
-	// The eventlistener waits until all the trigger executions (up-to the creation of the resources) and
-	// only when at least one of the execution completed successfully, it returns response code 201(Created) otherwise it returns 202 (Accepted).
-	code := http.StatusAccepted
-	for i := 0; i < len(triggers); i++ {
-		thiscode := <-result
-		// current take - if someone is doing unauthorized stuff, we abort immediately;
-		// unauthorized should be the final status code vs. the less than comparison
-		// below around accepted vs. created
-		if thiscode == http.StatusUnauthorized || thiscode == http.StatusForbidden {
-			code = thiscode
-			break
-		}
-		if thiscode < code {
-			code = thiscode
-		}
-	}
-
-	response.WriteHeader(code)
+	response.WriteHeader(http.StatusAccepted)
 	response.Header().Set("Content-Type", "application/json")
 	body := Response{
 		EventListener: r.EventListenerName,
@@ -227,19 +200,19 @@ func (r Sink) merge(et []triggersv1.EventListenerTrigger, trItems []*triggersv1.
 	return triggers, nil
 }
 
-func (r Sink) processTrigger(t triggersv1.Trigger, request *http.Request, event []byte, eventID string, eventLog *zap.SugaredLogger) error {
+func (r Sink) processTrigger(t triggersv1.Trigger, request *http.Request, event []byte, eventID string, eventLog *zap.SugaredLogger) {
 	log := eventLog.With(zap.String(triggersv1.TriggerLabelKey, t.Name))
 
 	finalPayload, header, iresp, err := r.ExecuteInterceptors(t, request, event, log, eventID)
 	if err != nil {
 		log.Error(err)
-		return err
+		return
 	}
 
 	if iresp != nil {
 		if !iresp.Continue {
 			log.Infof("interceptor stopped trigger processing: %v", iresp.Status.Err())
-			return iresp.Status.Err()
+			return
 		}
 	}
 
@@ -249,7 +222,7 @@ func (r Sink) processTrigger(t triggersv1.Trigger, request *http.Request, event
 		r.TriggerTemplateLister.TriggerTemplates(t.Namespace).Get)
 	if err != nil {
 		log.Error(err)
-		return err
+		return
 	}
 	extensions := map[string]interface{}{}
 	if iresp != nil && iresp.Extensions != nil {
@@ -258,18 +231,17 @@ func (r Sink) processTrigger(t triggersv1.Trigger, request *http.Request, event
 	params, err := template.ResolveParams(rt, finalPayload, header, extensions)
 	if err != nil {
 		log.Error(err)
-		return err
+		return
 	}
 
 	log.Infof("ResolvedParams : %+v", params)
 	resources := template.ResolveResources(rt.TriggerTemplate, params)
 
 	if err := r.CreateResources(t.Namespace, t.Spec.ServiceAccountName, resources, t.Name, eventID, log); err != nil {
 		log.Error(err)
-		return err
+		return
 	}
 	go r.recordResourceCreation(resources)
-	return nil
 }
 
 // ExecuteInterceptor executes all interceptors for the Trigger and returns back the body, header, and InterceptorResponse to use.