Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change InterceptorRequest.Body to string #877

Merged
merged 1 commit into from
Dec 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions pkg/apis/triggers/v1alpha1/interceptor_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package v1alpha1

import (
"context"
"encoding/json"
"fmt"
"strings"

Expand All @@ -20,10 +19,16 @@ type InterceptorInterface interface {
// Do not generate DeepCopy(). See #827
// +k8s:deepcopy-gen=false
type InterceptorRequest struct {
// Body is the incoming HTTP event body
Body json.RawMessage `json:"body,omitempty"`
// Body is the incoming HTTP event body. We use a "string" representation of the JSON body
// in order to preserve the body exactly as it was sent (including spaces etc.). This is necessary
dibyom marked this conversation as resolved.
Show resolved Hide resolved
// for some interceptors e.g. GitHub for validating the body with a signature. While []byte can also
// store an exact representation of the body, `json.Marshal` will compact []byte to a base64 encoded
// string which means that we will lose the spaces any time we marshal this struct.
Body string `json:"body,omitempty"`

// Header are the headers for the incoming HTTP event
Header map[string][]string `json:"header,omitempty"`

// Extensions are extra values that are added by previous interceptors in a chain
Extensions map[string]interface{} `json:"extensions,omitempty"`

Expand Down
2 changes: 1 addition & 1 deletion pkg/interceptors/bitbucket/bitbucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (w *Interceptor) Process(ctx context.Context, r *triggersv1.InterceptorRequ
}
secretToken := secret.Data[p.SecretRef.SecretKey]

if err := gh.ValidateSignature(header, r.Body, secretToken); err != nil {
if err := gh.ValidateSignature(header, []byte(r.Body), secretToken); err != nil {
return interceptors.Failf(codes.FailedPrecondition, err.Error())
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/interceptors/bitbucket/bitbucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestInterceptor_Process_ShouldContinue(t *testing.T) {
}

req := &triggersv1.InterceptorRequest{
Body: tt.payload,
Body: string(tt.payload),
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down Expand Up @@ -263,7 +263,7 @@ func TestInterceptor_Process_ShouldNotContinue(t *testing.T) {
}

req := &triggersv1.InterceptorRequest{
Body: tt.payload,
Body: string(tt.payload),
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down Expand Up @@ -308,7 +308,7 @@ func TestInterceptor_Process_InvalidParams(t *testing.T) {
}

req := &triggersv1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/interceptors/cel/cel.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ func (w *Interceptor) Process(ctx context.Context, r *triggersv1.InterceptorRequ
}

var payload = []byte(`{}`)
if r.Body != nil {
payload = r.Body
if r.Body != "" {
payload = []byte(r.Body)
}

evalContext, err := makeEvalContext(payload, r.Header, r.Context.EventURL, r.Extensions)
Expand Down
6 changes: 3 additions & 3 deletions pkg/interceptors/cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func TestInterceptor_Process(t *testing.T) {
Logger: logger.Sugar(),
}
res := w.Process(ctx, &triggersv1.InterceptorRequest{
Body: tt.body,
Body: string(tt.body),
Header: http.Header{
"Content-Type": []string{"application/json"},
"X-Test": []string{"test-value"},
Expand Down Expand Up @@ -356,7 +356,7 @@ func TestInterceptor_Process_Error(t *testing.T) {
Logger: logger.Sugar(),
}
res := w.Process(context.Background(), &triggersv1.InterceptorRequest{
Body: tt.body,
Body: string(tt.body),
Header: http.Header{
"Content-Type": []string{"application/json"},
"X-Test": []string{"test-value"},
Expand Down Expand Up @@ -392,7 +392,7 @@ func TestInterceptor_Process_InvalidParams(t *testing.T) {
Logger: logger.Sugar(),
}
res := w.Process(context.Background(), &triggersv1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: http.Header{},
InterceptorParams: map[string]interface{}{
"filter": func() {}, // Should fail JSON unmarshal
Expand Down
2 changes: 1 addition & 1 deletion pkg/interceptors/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (w *Interceptor) Process(ctx context.Context, r *triggersv1.InterceptorRequ
}
secretToken := secret.Data[p.SecretRef.SecretKey]

if err := gh.ValidateSignature(header, r.Body, secretToken); err != nil {
if err := gh.ValidateSignature(header, []byte(r.Body), secretToken); err != nil {
return interceptors.Fail(codes.FailedPrecondition, err.Error())
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/interceptors/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestInterceptor_ExecuteTrigger_Signature(t *testing.T) {
kubeClient := fakekubeclient.Get(ctx)

req := &triggersv1.InterceptorRequest{
Body: tt.payload,
Body: string(tt.payload),
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestInterceptor_ExecuteTrigger_ShouldNotContinue(t *testing.T) {
kubeClient := fakekubeclient.Get(ctx)

req := &triggersv1.InterceptorRequest{
Body: tt.payload,
Body: string(tt.payload),
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down Expand Up @@ -301,7 +301,7 @@ func TestInterceptor_ExecuteTrigger_with_invalid_content_type(t *testing.T) {
logger := zaptest.NewLogger(t)
kubeClient := fakekubeclient.Get(ctx)
req := &triggersv1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: http.Header{
"Content-Type": []string{"application/x-www-form-urlencoded"},
"X-Hub-Signature": []string{"foo"},
Expand Down Expand Up @@ -337,7 +337,7 @@ func TestInterceptor_Process_InvalidParams(t *testing.T) {
}

req := &triggersv1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down
7 changes: 3 additions & 4 deletions pkg/interceptors/gitlab/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package gitlab

import (
"context"
"encoding/json"
"net/http"
"testing"

Expand Down Expand Up @@ -99,7 +98,7 @@ func TestInterceptor_ExecuteTrigger_ShouldContinue(t *testing.T) {
logger := zaptest.NewLogger(t)
kubeClient := fakekubeclient.Get(ctx)
req := &triggersv1.InterceptorRequest{
Body: tt.payload,
Body: string(tt.payload),
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down Expand Up @@ -238,7 +237,7 @@ func TestInterceptor_ExecuteTrigger_ShouldNotContinue(t *testing.T) {
logger := zaptest.NewLogger(t)
kubeClient := fakekubeclient.Get(ctx)
req := &triggersv1.InterceptorRequest{
Body: tt.payload,
Body: string(tt.payload),
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down Expand Up @@ -286,7 +285,7 @@ func TestInterceptor_Process_InvalidParams(t *testing.T) {
}

req := &triggersv1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: http.Header{
"Content-Type": []string{"application/json"},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/interceptors/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestServer_ServeHTTP(t *testing.T) {
name: "valid request that should continue",
path: "/cel",
req: &v1alpha1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: map[string][]string{
"X-Event-Type": {"push"},
},
Expand All @@ -53,7 +53,7 @@ func TestServer_ServeHTTP(t *testing.T) {
name: "valid request that should not continue",
path: "/cel",
req: &v1alpha1.InterceptorRequest{
Body: json.RawMessage(`{}`),
Body: `{}`,
Header: map[string][]string{
"X-Event-Type": {"push"},
},
Expand Down
11 changes: 5 additions & 6 deletions pkg/sink/sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func (r Sink) ExecuteInterceptors(t triggersv1.Trigger, in *http.Request, event
// request is the request sent to the interceptors in the chain. Each interceptor can set the InterceptorParams field
// or add to the Extensions
request := triggersv1.InterceptorRequest{
Body: event,
Body: string(event),
Header: in.Header.Clone(),
Extensions: map[string]interface{}{}, // Empty extensions for the first interceptor in chain
Context: &triggersv1.TriggerContext{
Expand All @@ -266,9 +266,8 @@ func (r Sink) ExecuteInterceptors(t triggersv1.Trigger, in *http.Request, event
}

for _, i := range t.Spec.Interceptors {
if i.Webhook != nil {
// Old style interceptor (everything but CEL at the moment)
body, err := extendBodyWithExtensions(request.Body, request.Extensions)
if i.Webhook != nil { // Old style interceptor
body, err := extendBodyWithExtensions([]byte(request.Body), request.Extensions)
if err != nil {
return nil, nil, nil, fmt.Errorf("could not merge extensions with body: %w", err)
}
Expand All @@ -292,7 +291,7 @@ func (r Sink) ExecuteInterceptors(t triggersv1.Trigger, in *http.Request, event
// Set the next request to be the output of the last response to enable
// request chaining.
request.Header = res.Header.Clone()
request.Body = payload
request.Body = string(payload)
continue
}

Expand Down Expand Up @@ -329,7 +328,7 @@ func (r Sink) ExecuteInterceptors(t triggersv1.Trigger, in *http.Request, event
// Clear interceptorParams for the next interceptor in chain
request.InterceptorParams = map[string]interface{}{}
}
return request.Body, request.Header, &triggersv1.InterceptorResponse{
return []byte(request.Body), request.Header, &triggersv1.InterceptorResponse{
Continue: true,
Extensions: request.Extensions,
}, nil
Expand Down