Skip to content

Commit

Permalink
Adds annotation to disable payload validation in eventlistener
Browse files Browse the repository at this point in the history
This adds an annotation support tekton.dev/payload-validation
for eventlistener. If it is added to el with value as false then
the payload from events will not be validated and will be directly
passed to interceptors.
By default the payload validation is enabled. Only if annotation
is defined and its value is false then it will be disabled
for that particular el.

Signed-off-by: Shivam Mukhade [email protected]
  • Loading branch information
Shivam Mukhade authored and tekton-robot committed Aug 25, 2021
1 parent 125828e commit 48d8917
Show file tree
Hide file tree
Showing 15 changed files with 261 additions and 28 deletions.
1 change: 1 addition & 0 deletions cmd/eventlistenersink/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ func main() {
HTTPClient: http.DefaultClient,
EventListenerName: sinkArgs.ElName,
EventListenerNamespace: sinkArgs.ElNamespace,
PayloadValidation: sinkArgs.PayloadValidation,
Logger: logger,
Recorder: recorder,
Auth: sink.DefaultAuthOverride{},
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/triggers/v1alpha1/event_listener_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"fmt"

"github.com/tektoncd/triggers/pkg/apis/triggers"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
Expand All @@ -43,6 +44,11 @@ func (e *EventListener) Validate(ctx context.Context) *apis.FieldError {
// Since `el-` is added as the prefix of EventListener services, the name of EventListener must be no more than 60 characters long.
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("eventListener name '%s' must be no more than 60 characters long", e.ObjectMeta.Name), "metadata.name"))
}

if len(e.ObjectMeta.Annotations) != 0 {
errs = errs.Also(triggers.ValidateAnnotations(e.ObjectMeta.Annotations))
}

if apis.IsInDelete(ctx) {
return nil
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/apis/triggers/v1alpha1/event_listener_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/triggers/pkg/apis/triggers"
"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
"github.com/tektoncd/triggers/test"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -102,6 +103,20 @@ func Test_EventListenerValidate(t *testing.T) {
}},
},
},
}, {
name: "Valid EventListener with Annotation",
el: &v1alpha1.EventListener{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Annotations: map[string]string{triggers.PayloadValidationAnnotation: "true"},
},
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
TriggerRef: "tt",
}},
},
},
}, {
name: "Valid EventListener with TriggerBinding",
el: &v1alpha1.EventListener{
Expand Down Expand Up @@ -557,6 +572,20 @@ func TestEventListenerValidate_error(t *testing.T) {
}},
},
},
}, {
name: "Invalid Annotation value",
el: &v1alpha1.EventListener{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Annotations: map[string]string{triggers.PayloadValidationAnnotation: "xyz"},
},
Spec: v1alpha1.EventListenerSpec{
Triggers: []v1alpha1.EventListenerTrigger{{
TriggerRef: "tt",
}},
},
},
}, {
name: "TriggerBinding with no ref or spec",
el: &v1alpha1.EventListener{
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/triggers/v1beta1/event_listener_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"fmt"

"github.com/tektoncd/triggers/pkg/apis/triggers"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
Expand All @@ -43,6 +44,11 @@ func (e *EventListener) Validate(ctx context.Context) *apis.FieldError {
// Since `el-` is added as the prefix of EventListener services, the name of EventListener must be no more than 60 characters long.
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("eventListener name '%s' must be no more than 60 characters long", e.ObjectMeta.Name), "metadata.name"))
}

if len(e.GetObjectMeta().GetAnnotations()) != 0 {
errs = errs.Also(triggers.ValidateAnnotations(e.GetObjectMeta().GetAnnotations()))
}

if apis.IsInDelete(ctx) {
return nil
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/apis/triggers/v1beta1/event_listener_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

pipelinev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/triggers/pkg/apis/triggers"
triggersv1beta1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1beta1"
"github.com/tektoncd/triggers/test"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -102,6 +103,20 @@ func Test_EventListenerValidate(t *testing.T) {
}},
},
},
}, {
name: "Valid EventListener with Annotation",
el: &triggersv1beta1.EventListener{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Annotations: map[string]string{triggers.PayloadValidationAnnotation: "true"},
},
Spec: triggersv1beta1.EventListenerSpec{
Triggers: []triggersv1beta1.EventListenerTrigger{{
TriggerRef: "tt",
}},
},
},
}, {
name: "Valid EventListener with TriggerBinding",
el: &triggersv1beta1.EventListener{
Expand Down Expand Up @@ -557,6 +572,20 @@ func TestEventListenerValidate_error(t *testing.T) {
}},
},
},
}, {
name: "Invalid Annotation value",
el: &triggersv1beta1.EventListener{
ObjectMeta: metav1.ObjectMeta{
Name: "name",
Namespace: "namespace",
Annotations: map[string]string{triggers.PayloadValidationAnnotation: "xyz"},
},
Spec: triggersv1beta1.EventListenerSpec{
Triggers: []triggersv1beta1.EventListenerTrigger{{
TriggerRef: "tt",
}},
},
},
}, {
name: "TriggerBinding with no ref or spec",
el: &triggersv1beta1.EventListener{
Expand Down
39 changes: 39 additions & 0 deletions pkg/apis/triggers/validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
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 triggers

import (
"fmt"

"knative.dev/pkg/apis"
)

const (
PayloadValidationAnnotation = "tekton.dev/payload-validation"
)

func ValidateAnnotations(annotations map[string]string) *apis.FieldError {
var errs *apis.FieldError

if value, ok := annotations[PayloadValidationAnnotation]; ok {
if value != "true" && value != "false" {
errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%s annotation must have value 'true' or 'false'", PayloadValidationAnnotation), "metadata.annotations"))
}
}

return errs
}
37 changes: 37 additions & 0 deletions pkg/apis/triggers/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
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 triggers

import (
"testing"
)

func Test_PayloadValidationAnnotation_Valid(t *testing.T) {
annotations := map[string]string{PayloadValidationAnnotation: "false"}
err := ValidateAnnotations(annotations)
if err != nil {
t.Errorf("Unexpected Error: %v", err)
}
}

func Test_PayloadValidationAnnotation_InvalidValue(t *testing.T) {
annotations := map[string]string{PayloadValidationAnnotation: "abc"}
err := ValidateAnnotations(annotations)
if err == nil {
t.Errorf("Expected Error but got nil")
}
}
9 changes: 9 additions & 0 deletions pkg/reconciler/eventlistener/eventlistener.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/triggers/pkg/apis/triggers"
"github.com/tektoncd/triggers/pkg/apis/triggers/contexts"
"github.com/tektoncd/triggers/pkg/apis/triggers/v1beta1"
triggersclientset "github.com/tektoncd/triggers/pkg/client/clientset/versioned"
Expand Down Expand Up @@ -883,6 +884,13 @@ func getContainer(el *v1beta1.EventListener, c Config, pod *duckv1.WithPod) core
isMultiNS = true
}

payloadValidation := true
if value, ok := el.GetAnnotations()[triggers.PayloadValidationAnnotation]; ok {
if value == "false" {
payloadValidation = false
}
}

return corev1.Container{
Name: "event-listener",
Image: *c.Image,
Expand All @@ -900,6 +908,7 @@ func getContainer(el *v1beta1.EventListener, c Config, pod *duckv1.WithPod) core
"--idletimeout=" + strconv.FormatInt(*c.IdleTimeOut, 10),
"--timeouthandler=" + strconv.FormatInt(*c.TimeOutHandler, 10),
"--is-multi-ns=" + strconv.FormatBool(isMultiNS),
"--payload-validation=" + strconv.FormatBool(payloadValidation),
},
Env: env,
}
Expand Down
25 changes: 15 additions & 10 deletions pkg/sink/initialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ var (
"The filename for the TLS certificate.")
tlsKeyFlag = flag.String("tls-key", "",
"The filename for the TLS key.")
payloadValidation = flag.Bool("payload-validation", true,
"Whether to disable payload validation or not.")
)

// Args define the arguments for Sink.
Expand All @@ -81,6 +83,8 @@ type Args struct {
Key string
// Cert defines the filename for tls Cert.
Cert string
// PayloadValidation defines whether to validate payload or not
PayloadValidation bool
}

// Clients define the set of client dependencies Sink requires.
Expand All @@ -104,16 +108,17 @@ func GetArgs() (Args, error) {
}

return Args{
ElName: *nameFlag,
ElNamespace: *namespaceFlag,
Port: *portFlag,
IsMultiNS: *isMultiNSFlag,
ELReadTimeOut: time.Duration(*elReadTimeOut),
ELWriteTimeOut: time.Duration(*elWriteTimeOut),
ELIdleTimeOut: time.Duration(*elIdleTimeOut),
ELTimeOutHandler: time.Duration(*elTimeOutHandler),
Cert: *tlsCertFlag,
Key: *tlsKeyFlag,
ElName: *nameFlag,
ElNamespace: *namespaceFlag,
Port: *portFlag,
IsMultiNS: *isMultiNSFlag,
PayloadValidation: *payloadValidation,
ELReadTimeOut: time.Duration(*elReadTimeOut),
ELWriteTimeOut: time.Duration(*elWriteTimeOut),
ELIdleTimeOut: time.Duration(*elIdleTimeOut),
ELTimeOutHandler: time.Duration(*elTimeOutHandler),
Cert: *tlsCertFlag,
Key: *tlsKeyFlag,
}, nil
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/sink/initialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ func Test_GetArgs(t *testing.T) {
if sinkArgs.IsMultiNS != true {
t.Errorf("Error EL Type want type, got %t", sinkArgs.IsMultiNS)
}
if sinkArgs.PayloadValidation != true {
t.Errorf("Error EL PayloadValidation want true, got %t", sinkArgs.PayloadValidation)
}
}

func Test_GetArgs_error(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sink/sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Sink struct {
Logger *zap.SugaredLogger
Recorder *Recorder
Auth AuthOverride

PayloadValidation bool
// WGProcessTriggers keeps track of triggers currently being processed
// Currently only used in tests to wait for all triggers to finish processing
WGProcessTriggers *sync.WaitGroup
Expand Down
1 change: 1 addition & 0 deletions pkg/sink/sink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ func getSinkAssets(t *testing.T, resources test.Resources, elName string, webhoo
ClusterTriggerBindingLister: clustertriggerbindinginformer.Get(ctx).Lister(),
TriggerTemplateLister: triggertemplateinformer.Get(ctx).Lister(),
ClusterInterceptorLister: interceptorinformer.Get(ctx).Lister(),
PayloadValidation: true,
}
return r, dynamicClient
}
Expand Down
32 changes: 17 additions & 15 deletions pkg/sink/validate_payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,24 @@ func (r Sink) IsValidPayload(eventHandler http.Handler) http.Handler {
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.recordCountMetrics(failTag)
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)
if r.PayloadValidation {
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.recordCountMetrics(failTag)
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
}
return
}
eventHandler.ServeHTTP(response, request)
})
Expand Down
Loading

0 comments on commit 48d8917

Please sign in to comment.