From f90ab9681aac89be8f4e663e5ae435008d49cc8c Mon Sep 17 00:00:00 2001 From: Khurram Baig Date: Wed, 24 Feb 2021 19:43:19 +0530 Subject: [PATCH] Validate Event Body for Json Format Throw Error Message and Bad Request Code if event Body isn't json. --- cmd/eventlistenersink/main.go | 3 +- pkg/sink/sink.go | 2 + pkg/sink/validate_payload.go | 54 ++++++++++++++++++ pkg/sink/validate_payload_test.go | 92 +++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 pkg/sink/validate_payload.go create mode 100644 pkg/sink/validate_payload_test.go diff --git a/cmd/eventlistenersink/main.go b/cmd/eventlistenersink/main.go index 538c8701b..d3d3fcac2 100644 --- a/cmd/eventlistenersink/main.go +++ b/cmd/eventlistenersink/main.go @@ -118,7 +118,8 @@ func main() { // Listen and serve logger.Infof("Listen and serve on port %s", sinkArgs.Port) mux := http.NewServeMux() - mux.HandleFunc("/", r.HandleEvent) + eventHandler := http.HandlerFunc(r.HandleEvent) + mux.Handle("/", r.IsValidPayload(eventHandler)) // For handling Liveness Probe // TODO(dibyom): Livness, metrics etc. should be on a separate port diff --git a/pkg/sink/sink.go b/pkg/sink/sink.go index 9476ffb85..ff9cc7ef0 100644 --- a/pkg/sink/sink.go +++ b/pkg/sink/sink.go @@ -71,6 +71,8 @@ type Response struct { Namespace string `json:"namespace,omitempty"` // 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 + ErrorMessage string `json:"errorMessage,omitempty"` } // HandleEvent processes an incoming HTTP event for the event listener. diff --git a/pkg/sink/validate_payload.go b/pkg/sink/validate_payload.go new file mode 100644 index 000000000..e68e74cff --- /dev/null +++ b/pkg/sink/validate_payload.go @@ -0,0 +1,54 @@ +/* +Copyright 2021 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sink + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" +) + +func (r Sink) IsValidPayload(eventHandler http.Handler) http.Handler { + return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + payload, err := ioutil.ReadAll(request.Body) + request.Body = ioutil.NopCloser(bytes.NewBuffer(payload)) + if err != nil { + r.Logger.Errorf("Error reading event body: %s", err) + response.WriteHeader(http.StatusInternalServerError) + return + } + var event map[string]interface{} + if err := json.Unmarshal([]byte(payload), &event); err != nil { + errMsg := fmt.Sprintf("Invalid event body format format: %s", err) + r.Logger.Error(errMsg) + response.WriteHeader(http.StatusBadRequest) + response.Header().Set("Content-Type", "application/json") + body := Response{ + EventListener: r.EventListenerName, + Namespace: r.EventListenerNamespace, + ErrorMessage: errMsg, + } + if err := json.NewEncoder(response).Encode(body); err != nil { + r.Logger.Errorf("failed to write back sink response: %v", err) + } + return + } + eventHandler.ServeHTTP(response, request) + }) +} diff --git a/pkg/sink/validate_payload_test.go b/pkg/sink/validate_payload_test.go new file mode 100644 index 000000000..3a54d388f --- /dev/null +++ b/pkg/sink/validate_payload_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2021 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sink + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1" + "github.com/tektoncd/triggers/test" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestSink_IsValidPayload(t *testing.T) { + const defaultELName = "test-el" + for _, tc := range []struct { + name string + testResources test.Resources + eventBody []byte + wantStatusCode int + }{{ + name: "event with Json Body", + testResources: test.Resources{ + EventListeners: []*triggersv1.EventListener{{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultELName, + Namespace: namespace, + }, + Spec: triggersv1.EventListenerSpec{ + Triggers: []triggersv1.EventListenerTrigger{{ + TriggerRef: "test", + }}, + }, + }}, + }, + eventBody: json.RawMessage(`{"head_commit": {"id": "testrevision"}, "repository": {"url": "testurl"}}`), + wantStatusCode: http.StatusAccepted, + }, { + name: "event with non Json Body", + testResources: test.Resources{ + EventListeners: []*triggersv1.EventListener{{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultELName, + Namespace: namespace, + }, + Spec: triggersv1.EventListenerSpec{ + Triggers: []triggersv1.EventListenerTrigger{{ + TriggerRef: "test", + }}, + }, + }}, + }, + eventBody: []byte(`xml`), + wantStatusCode: http.StatusBadRequest, + }} { + t.Run(tc.name, func(t *testing.T) { + elName := defaultELName + if len(tc.testResources.EventListeners) > 0 { + elName = tc.testResources.EventListeners[0].Name + } + sink, _ := getSinkAssets(t, tc.testResources, elName, nil) + + ts := httptest.NewServer(sink.IsValidPayload(http.HandlerFunc(sink.HandleEvent))) + defer ts.Close() + + resp, err := http.Post(ts.URL, "application/json", bytes.NewReader(tc.eventBody)) + if err != nil { + t.Fatalf("error making request to eventListener: %s", err) + } + if resp.StatusCode != tc.wantStatusCode { + t.Fatalf("Status code mismatch: got %d, want %d", resp.StatusCode, http.StatusInternalServerError) + } + }) + } +}