Add EventListener UID to sink response, mark name/namespace as deprec…

…ated. 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.
tektoncd · May 12, 2021 · 368e922 · 368e922
1 parent 74dec96
commit 368e922
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 {