From 368e92221d1045e93ec6308984d7e5c7fda17740 Mon Sep 17 00:00:00 2001 From: Billy Lynch Date: Tue, 11 May 2021 13:23:13 -0400 Subject: [PATCH] Add EventListener UID to sink response, mark name/namespace as deprecated. The goal here is to provide a more opaque identifier to name/namespace, which might contain PII in response. We will continue to support name/namespace for the time being, but mark the fields as deprecated to be removed in a later release. This change also includes minor clean up to log annotations for common identifiers. --- docs/eventlisteners.md | 20 ++++++++++++++++---- pkg/sink/sink.go | 38 ++++++++++++++++++++++++-------------- pkg/sink/sink_test.go | 19 ++++++++++++++++--- test/eventlistener_test.go | 5 +++-- 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/docs/eventlisteners.md b/docs/eventlisteners.md index 56e6577225..b44ddec52a 100644 --- a/docs/eventlisteners.md +++ b/docs/eventlisteners.md @@ -375,13 +375,25 @@ Otherwise, it responds with a `202 ACCEPTED` HTTP response. After detecting an event, the `EventListener` responds with the following message: -```JSON -{"eventListener":"listener","namespace":"default","eventID":"h2bb7"} +```json +{ + "eventListener": "listener", + "namespace": "default", + "eventListenerUID": "ea71a6e4-9531-43a1-94fe-6136515d938c", + "eventID": "14a657c3-6816-45bf-b214-4afdaefc4ebd" +} ``` -- `eventListener` - name of the target EventListener -- `namespace` - namespace of the target EventListener + +- `eventListenerUID` - [UID](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids) of the target EventListener. - `eventID` - UID assigned to this event request +### Deprecated Fields + +These fields are included in `EventListener` responses, but will be removed in a future release. + +- `eventListener` - name of the target EventListener. Use `eventListenerUID` instead. +- `namespace` - namespace of the target EventListener. Use `eventListenerUID` instead. + ## TLS HTTPS support in `EventListeners` Tekton Triggers supports both HTTP and TLS-based HTTPS connections. To configure your `EventListener` for TLS, diff --git a/pkg/sink/sink.go b/pkg/sink/sink.go index 86fb18c64a..343bb2feca 100644 --- a/pkg/sink/sink.go +++ b/pkg/sink/sink.go @@ -67,10 +67,14 @@ type Sink struct { // Response defines the HTTP body that the Sink responds to events with. type Response struct { - // EventListener is the name of the eventListener + // EventListener is the name of the eventListener. + // Deprecated: use EventListenerUID instead. EventListener string `json:"eventListener"` - // Namespace is the namespace that the eventListener is running in + // Namespace is the namespace that the eventListener is running in. + // Deprecated: use EventListenerUID instead. Namespace string `json:"namespace,omitempty"` + // EventListenerUID is the UID of the EventListener + EventListenerUID string `json:"eventListenerUID"` // EventID is a uniqueID that gets assigned to each incoming request EventID string `json:"eventID,omitempty"` // ErrorMessage gives message about Error which occurs during event processing @@ -79,23 +83,28 @@ type Response struct { // HandleEvent processes an incoming HTTP event for the event listener. func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) { + log := r.Logger.With( + zap.String("eventlistener", r.EventListenerName), + zap.String("namespace", r.EventListenerNamespace), + ) el, err := r.EventListenerLister.EventListeners(r.EventListenerNamespace).Get(r.EventListenerName) if err != nil { - r.Logger.Errorf("Error getting EventListener %s in Namespace %s: %s", r.EventListenerName, r.EventListenerNamespace, err) + log.Errorf("Error getting EventListener %s in Namespace %s: %s", r.EventListenerName, r.EventListenerNamespace, err) response.WriteHeader(http.StatusInternalServerError) return } + elUID := string(el.GetUID()) + log = log.With(zap.String("eventlistenerUID", elUID)) event, err := ioutil.ReadAll(request.Body) if err != nil { - r.Logger.Errorf("Error reading event body: %s", err) + log.Errorf("Error reading event body: %s", err) response.WriteHeader(http.StatusInternalServerError) return } eventID := template.UUID() - eventLog := r.Logger.With(zap.String(triggersv1.EventIDLabelKey, eventID)) - eventLog.Debugf("EventListener: %s in Namespace: %s handling event (EventID: %s) with path %s, payload: %s and header: %v", - r.EventListenerName, r.EventListenerNamespace, eventID, request.URL.Path, string(event), request.Header) + log = log.With(zap.String(triggersv1.EventIDLabelKey, eventID)) + log.Debugf("handling event with path %s, payload: %s and header: %v", request.URL.Path, string(event), request.Header) var trItems []*triggersv1.Trigger labelSelector := labels.Everything() if el.Spec.LabelSelector != nil { @@ -134,7 +143,7 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) { if triggerFunc != nil { trList, err := triggerFunc() if err != nil { - r.Logger.Errorf("Error getting Triggers: %s", err) + log.Errorf("Error getting Triggers: %s", err) response.WriteHeader(http.StatusInternalServerError) return } @@ -143,7 +152,7 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) { triggers, err := r.merge(el.Spec.Triggers, trItems) if err != nil { - r.Logger.Errorf("Error merging Triggers: %s", err) + log.Errorf("Error merging Triggers: %s", err) response.WriteHeader(http.StatusInternalServerError) return } @@ -152,7 +161,7 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) { for _, t := range triggers { go func(t triggersv1.Trigger) { localRequest := request.Clone(request.Context()) - if err := r.processTrigger(t, localRequest, event, eventID, eventLog); err != nil { + if err := r.processTrigger(t, localRequest, event, eventID, log); err != nil { if kerrors.IsUnauthorized(err) { result <- http.StatusUnauthorized return @@ -188,12 +197,13 @@ func (r Sink) HandleEvent(response http.ResponseWriter, request *http.Request) { response.WriteHeader(code) response.Header().Set("Content-Type", "application/json") body := Response{ - EventListener: r.EventListenerName, - Namespace: r.EventListenerNamespace, - EventID: eventID, + EventListener: r.EventListenerName, + EventListenerUID: elUID, + Namespace: r.EventListenerNamespace, + EventID: eventID, } if err := json.NewEncoder(response).Encode(body); err != nil { - eventLog.Errorf("failed to write back sink response: %v", err) + log.Errorf("failed to write back sink response: %v", err) } } diff --git a/pkg/sink/sink_test.go b/pkg/sink/sink_test.go index 3a64ace2cd..153b2fae95 100644 --- a/pkg/sink/sink_test.go +++ b/pkg/sink/sink_test.go @@ -30,6 +30,7 @@ import ( "testing" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -70,6 +71,7 @@ import ( const ( eventID = "12345" namespace = "foo" + elUID = "el-uid" ) func init() { @@ -221,9 +223,10 @@ func checkSinkResponse(t *testing.T, resp *http.Response, elName string) { t.Fatalf("Error reading response body: %s", err) } wantBody := Response{ - EventListener: elName, - Namespace: namespace, - EventID: eventID, + EventListener: elName, + EventListenerUID: elUID, + Namespace: namespace, + EventID: eventID, } if diff := cmp.Diff(wantBody, gotBody); diff != "" { t.Errorf("did not get expected response back -want,+got: %s", diff) @@ -389,6 +392,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ Triggers: []triggersv1.EventListenerTrigger{{ @@ -452,6 +456,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ NamespaceSelector: triggersv1.NamespaceSelector{ @@ -521,6 +526,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ NamespaceSelector: triggersv1.NamespaceSelector{ @@ -594,6 +600,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ LabelSelector: &metav1.LabelSelector{ @@ -624,6 +631,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ Triggers: []triggersv1.EventListenerTrigger{{ @@ -651,6 +659,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ Triggers: []triggersv1.EventListenerTrigger{{ @@ -708,6 +717,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ Triggers: []triggersv1.EventListenerTrigger{{ @@ -762,6 +772,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ Triggers: []triggersv1.EventListenerTrigger{{ @@ -783,6 +794,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ NamespaceSelector: triggersv1.NamespaceSelector{ @@ -831,6 +843,7 @@ func TestHandleEvent(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: eventListenerName, Namespace: namespace, + UID: types.UID(elUID), }, Spec: triggersv1.EventListenerSpec{ Triggers: []triggersv1.EventListenerTrigger{{ diff --git a/test/eventlistener_test.go b/test/eventlistener_test.go index 6ab598b33f..8c944b715e 100644 --- a/test/eventlistener_test.go +++ b/test/eventlistener_test.go @@ -450,8 +450,9 @@ func TestEventListenerCreate(t *testing.T) { t.Errorf("sink did not return 2xx response. Got status code: %d", resp.StatusCode) } wantBody := sink.Response{ - EventListener: "my-eventlistener", - Namespace: namespace, + EventListener: "my-eventlistener", + Namespace: namespace, + EventListenerUID: string(el.UID()), } var gotBody sink.Response if err := json.NewDecoder(resp.Body).Decode(&gotBody); err != nil {