From eac6a23eef561bb2c9840019b54afa4518a0e6fe Mon Sep 17 00:00:00 2001 From: Atomys Date: Fri, 8 Mar 2024 02:01:29 +0100 Subject: [PATCH] feat: allow customization of the response (#177) **Describe the pull request** This pull request introduce the possibility to set the response of the webhook per spec using the formatting feature already present in the app. **Checklist** - [x] I have linked the relative issue to this pull request - [x] I have made the modifications or added tests related to my PR - [x] I have added/updated the documentation for my RP - [x] I put my PR in Ready for Review only when all the checklist is checked **Breaking changes ?** no --- .github/workflows/k6.yaml | 2 +- README.md | 17 +++++++ config/webhooked.example.yaml | 7 ++- internal/config/configuration.go | 19 ++++--- internal/config/configuration_test.go | 4 +- internal/config/structs.go | 16 ++++++ internal/server/v1alpha1/handlers.go | 49 +++++++++++++------ internal/server/v1alpha1/handlers_test.go | 46 ++++++++++++++--- pkg/formatting/formatter.go | 8 +-- pkg/formatting/formatter_test.go | 10 ++++ pkg/formatting/functions.go | 37 +++++++++++++- pkg/formatting/functions_test.go | 45 +++++++++++++++++ tests/integrations/options.js | 6 ++- tests/integrations/scenarios.js | 30 +++++++++--- .../webhooked_config.integration.yaml | 31 ++++++++++++ tests/webhooks.tests.yaml | 6 ++- 16 files changed, 286 insertions(+), 47 deletions(-) diff --git a/.github/workflows/k6.yaml b/.github/workflows/k6.yaml index 55992eb..ba77eb4 100644 --- a/.github/workflows/k6.yaml +++ b/.github/workflows/k6.yaml @@ -27,7 +27,7 @@ jobs: check-latest: true - name: Install k6 run: | - curl https://github.com/grafana/k6/releases/download/v0.45.0/k6-v0.45.0-linux-amd64.tar.gz -L | tar xvz --strip-components 1 + curl https://github.com/grafana/k6/releases/download/v0.49.0/k6-v0.49.0-linux-amd64.tar.gz -L | tar xvz --strip-components 1 - name: Start application and run K6 continue-on-error: true run: | diff --git a/README.md b/README.md index bb974f5..3b9547f 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,23 @@ specs: port: 6379 database: 0 key: example-webhook + + + # Response is the final step of the pipeline. It allows you to send a response + # to the webhook sender. You can use the built-in helper function to format it + # as you want. (Optional) + # + # In this example we send a JSON response with a 200 HTTP code and a custom + # content type header `application/json`. The response contains the deliveryID + # header value or `unknown` if not present in the request. + response: + formatting: + templateString: | + { + "deliveryID": "{{ .Request.Header | getHeader "X-Delivery" | default "unknown" }}" + } + httpCode: 200 + contentType: application/json ``` More informations about security pipeline available on wiki : [Configuration/Security](https://github.com/42Atomys/webhooked/wiki/Security) diff --git a/config/webhooked.example.yaml b/config/webhooked.example.yaml index 650232c..89cc2e5 100644 --- a/config/webhooked.example.yaml +++ b/config/webhooked.example.yaml @@ -25,4 +25,9 @@ specs: password: valueFrom: envRef: REDIS_PASSWORD - key: example-webhook \ No newline at end of file + key: example-webhook + response: + formatting: + templateString: '{ "status": "ok" }' + httpCode: 200 + contentType: application/json \ No newline at end of file diff --git a/internal/config/configuration.go b/internal/config/configuration.go index f4e0328..6bd41cd 100644 --- a/internal/config/configuration.go +++ b/internal/config/configuration.go @@ -23,9 +23,12 @@ var ( currentConfig = &Configuration{} // ErrSpecNotFound is returned when the spec is not found ErrSpecNotFound = errors.New("spec not found") - // defaultTemplate is the default template for the payload + // defaultPayloadTemplate is the default template for the payload // when no template is defined - defaultTemplate = `{{ .Payload }}` + defaultPayloadTemplate = `{{ .Payload }}` + // defaultResponseTemplate is the default template for the response + // when no template is defined + defaultResponseTemplate = `` ) // Load loads the configuration from the configuration file @@ -73,13 +76,17 @@ func Load(cfgFile string) error { return err } - if spec.Formatting, err = loadTemplate(spec.Formatting, nil); err != nil { + if spec.Formatting, err = loadTemplate(spec.Formatting, nil, defaultPayloadTemplate); err != nil { return fmt.Errorf("configured storage for %s received an error: %s", spec.Name, err.Error()) } if err = loadStorage(spec); err != nil { return fmt.Errorf("configured storage for %s received an error: %s", spec.Name, err.Error()) } + + if spec.Response.Formatting, err = loadTemplate(spec.Response.Formatting, nil, defaultResponseTemplate); err != nil { + return fmt.Errorf("configured response for %s received an error: %s", spec.Name, err.Error()) + } } log.Info().Msgf("Load %d configurations", len(currentConfig.Specs)) @@ -143,7 +150,7 @@ func loadStorage(spec *WebhookSpec) (err error) { return fmt.Errorf("storage %s cannot be loaded properly: %s", s.Type, err.Error()) } - if s.Formatting, err = loadTemplate(s.Formatting, spec.Formatting); err != nil { + if s.Formatting, err = loadTemplate(s.Formatting, spec.Formatting, defaultPayloadTemplate); err != nil { return fmt.Errorf("storage %s cannot be loaded properly: %s", s.Type, err.Error()) } } @@ -155,7 +162,7 @@ func loadStorage(spec *WebhookSpec) (err error) { // loadTemplate loads the template for the given `spec`. When no spec is defined // we try to load the template from the parentSpec and fallback to the default // template if parentSpec is not given. -func loadTemplate(spec, parentSpec *FormattingSpec) (*FormattingSpec, error) { +func loadTemplate(spec, parentSpec *FormattingSpec, defaultTemplate string) (*FormattingSpec, error) { if spec == nil { spec = &FormattingSpec{} } @@ -185,7 +192,7 @@ func loadTemplate(spec, parentSpec *FormattingSpec) (*FormattingSpec, error) { if parentSpec != nil { if parentSpec.Template == "" { var err error - parentSpec, err = loadTemplate(parentSpec, nil) + parentSpec, err = loadTemplate(parentSpec, nil, defaultTemplate) if err != nil { return spec, err } diff --git a/internal/config/configuration_test.go b/internal/config/configuration_test.go index 8822ff3..7cad635 100644 --- a/internal/config/configuration_test.go +++ b/internal/config/configuration_test.go @@ -263,7 +263,7 @@ func Test_loadTemplate(t *testing.T) { nil, nil, false, - defaultTemplate, + defaultPayloadTemplate, }, { "template string", @@ -317,7 +317,7 @@ func Test_loadTemplate(t *testing.T) { } for _, test := range tests { - tmpl, err := loadTemplate(test.input, test.parentSpec) + tmpl, err := loadTemplate(test.input, test.parentSpec, defaultPayloadTemplate) if test.wantErr { assert.Error(t, err, test.name) } else { diff --git a/internal/config/structs.go b/internal/config/structs.go index 369d041..e98c41e 100644 --- a/internal/config/structs.go +++ b/internal/config/structs.go @@ -52,6 +52,22 @@ type WebhookSpec struct { // Storage is the configuration for the storage of the webhook spec // It is defined by the user and can be empty. Storage []*StorageSpec `mapstructure:"storage" json:"-"` + // Response is the configuration for the response of the webhook sent + // to the caller. It is defined by the user and can be empty. + Response ResponseSpec `mapstructure:"response" json:"-"` +} + +type ResponseSpec struct { + // Formatting is used to define the response body sent by webhooked + // to the webhook caller. When this configuration is empty, no response + // body is sent. It is defined by the user and can be empty. + Formatting *FormattingSpec `mapstructure:"formatting" json:"-"` + // HTTPCode is the HTTP code of the response. It is defined by the user + // and can be empty. (default: 200) + HttpCode int `mapstructure:"httpCode" json:"httpCode"` + // ContentType is the content type of the response. It is defined by the user + // and can be empty. (default: plain/text) + ContentType string `mapstructure:"contentType" json:"contentType"` } // Security is the struct contains the configuration for a security diff --git a/internal/server/v1alpha1/handlers.go b/internal/server/v1alpha1/handlers.go index f97f29b..aa0682e 100644 --- a/internal/server/v1alpha1/handlers.go +++ b/internal/server/v1alpha1/handlers.go @@ -21,7 +21,7 @@ type Server struct { // config is the current configuration of the server config *config.Configuration // webhookService is the function that will be called to process the webhook - webhookService func(s *Server, spec *config.WebhookSpec, r *http.Request) error + webhookService func(s *Server, spec *config.WebhookSpec, r *http.Request) (string, error) // logger is the logger used by the server logger zerolog.Logger } @@ -68,7 +68,8 @@ func (s *Server) WebhookHandler() http.HandlerFunc { return } - if err := s.webhookService(s, spec, r); err != nil { + responseBody, err := s.webhookService(s, spec, r) + if err != nil { switch err { case errSecurityFailed: w.WriteHeader(http.StatusForbidden) @@ -79,6 +80,22 @@ func (s *Server) WebhookHandler() http.HandlerFunc { return } } + + if responseBody != "" { + log.Debug().Str("response", responseBody).Msg("Webhook response") + if _, err := w.Write([]byte(responseBody)); err != nil { + s.logger.Error().Err(err).Msg("Error during response writing") + } + } + + if spec.Response.HttpCode != 0 { + w.WriteHeader(spec.Response.HttpCode) + } + + if spec.Response.ContentType != "" { + w.Header().Set("Content-Type", spec.Response.ContentType) + } + s.logger.Debug().Str("entry", spec.Name).Msg("Webhook processed successfully") } } @@ -86,26 +103,26 @@ func (s *Server) WebhookHandler() http.HandlerFunc { // webhookService is the function that will be called to process the webhook call // it will call the security pipeline if configured and store data on each configured // storages -func webhookService(s *Server, spec *config.WebhookSpec, r *http.Request) (err error) { +func webhookService(s *Server, spec *config.WebhookSpec, r *http.Request) (responseTemplare string, err error) { ctx := r.Context() if spec == nil { - return config.ErrSpecNotFound + return "", config.ErrSpecNotFound } if r.Body == nil { - return errRequestBodyMissing + return "", errRequestBodyMissing } defer r.Body.Close() data, err := io.ReadAll(r.Body) if err != nil { - return err + return "", err } if spec.HasSecurity() { if err := s.runSecurity(spec, r, data); err != nil { - return err + return "", err } } @@ -117,26 +134,30 @@ func webhookService(s *Server, spec *config.WebhookSpec, r *http.Request) (err e WithData("Config", config.Current()) for _, storage := range spec.Storage { - payloadFormatter = payloadFormatter.WithData("Storage", storage) + storageFormatter := *payloadFormatter.WithData("Storage", storage) - storagePayload, err := payloadFormatter.WithTemplate(storage.Formatting.Template).Render() + storagePayload, err := storageFormatter.WithTemplate(storage.Formatting.Template).Render() if err != nil { - return err + return "", err } // update the formatter with the rendered payload of storage formatting // this will allow to chain formatting - payloadFormatter.WithData("PreviousPayload", previousPayload) - ctx = formatting.ToContext(ctx, payloadFormatter) + storageFormatter.WithData("PreviousPayload", previousPayload) + ctx = formatting.ToContext(ctx, &storageFormatter) log.Debug().Msgf("store following data: %s", storagePayload) if err := storage.Client.Push(ctx, []byte(storagePayload)); err != nil { - return err + return "", err } log.Debug().Str("storage", storage.Client.Name()).Msgf("stored successfully") } - return err + if spec.Response.Formatting != nil && spec.Response.Formatting.Template != "" { + return payloadFormatter.WithTemplate(spec.Response.Formatting.Template).Render() + } + + return "", err } // runSecurity will run the security pipeline for the current webhook call diff --git a/internal/server/v1alpha1/handlers_test.go b/internal/server/v1alpha1/handlers_test.go index ec334bf..1d435f9 100644 --- a/internal/server/v1alpha1/handlers_test.go +++ b/internal/server/v1alpha1/handlers_test.go @@ -52,7 +52,7 @@ func TestServer_WebhookHandler(t *testing.T) { EntrypointURL: "/test", }}, }, - webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) error { return expectedError }, + webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) (string, error) { return "", expectedError }, }).Code, ) @@ -67,7 +67,27 @@ func TestServer_WebhookHandler(t *testing.T) { EntrypointURL: "/test", }}, }, - webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) error { return nil }, + webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) (string, error) { return "", nil }, + }).Code, + ) + + assert.Equal(t, + http.StatusOK, + testServerWebhookHandlerHelper(t, &Server{ + config: &config.Configuration{ + APIVersion: "v1alpha1", + Specs: []*config.WebhookSpec{ + { + Name: "test", + EntrypointURL: "/test", + Response: config.ResponseSpec{ + Formatting: &config.FormattingSpec{Template: "test-payload"}, + HttpCode: 200, + ContentType: "application/json", + }, + }}, + }, + webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) (string, error) { return "test-payload", nil }, }).Code, ) @@ -82,7 +102,9 @@ func TestServer_WebhookHandler(t *testing.T) { EntrypointURL: "/test", }}, }, - webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) error { return errSecurityFailed }, + webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) (string, error) { + return "", errSecurityFailed + }, }).Code, ) @@ -97,7 +119,7 @@ func TestServer_WebhookHandler(t *testing.T) { EntrypointURL: "/test", }}, }, - webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) error { return nil }, + webhookService: func(s *Server, spec *config.WebhookSpec, r *http.Request) (string, error) { return "", nil }, }).Code, ) } @@ -162,13 +184,23 @@ func Test_webhookService(t *testing.T) { {"empty security", &input{&config.WebhookSpec{ SecurityPipeline: factory.NewPipeline(), }, req}, false, nil}, - {"valid security", &input{&config.WebhookSpec{ SecurityPipeline: validPipeline, }, req}, false, nil}, {"invalid security", &input{&config.WebhookSpec{ SecurityPipeline: invalidPipeline, }, req}, true, errSecurityFailed}, + {"valid payload with response", &input{ + &config.WebhookSpec{ + SecurityPipeline: validPipeline, + Response: config.ResponseSpec{ + Formatting: &config.FormattingSpec{Template: "{{.Payload}}"}, + HttpCode: 200, + ContentType: "application/json", + }, + }, + req, + }, false, nil}, {"invalid body payload", &input{&config.WebhookSpec{ SecurityPipeline: validPipeline, }, invalidReq}, true, errRequestBodyMissing}, @@ -176,7 +208,7 @@ func Test_webhookService(t *testing.T) { for _, test := range tests { log.Warn().Msgf("body %+v", test.input.req.Body) - got := webhookService(&Server{}, test.input.spec, test.input.req) + _, got := webhookService(&Server{}, test.input.spec, test.input.req) if test.wantErr { assert.ErrorIs(got, test.matchErr, "input: %s", test.name) } else { @@ -233,7 +265,7 @@ func TestServer_webhokServiceStorage(t *testing.T) { }, } - got := webhookService(&Server{}, spec, test.req) + _, got := webhookService(&Server{}, spec, test.req) if test.wantErr { assert.Error(t, got, "input: %s", test.name) } else { diff --git a/pkg/formatting/formatter.go b/pkg/formatting/formatter.go index d32dd10..6bfe078 100644 --- a/pkg/formatting/formatter.go +++ b/pkg/formatting/formatter.go @@ -7,8 +7,6 @@ import ( "net/http" "sync" "text/template" - - "github.com/rs/zerolog/log" ) type Formatter struct { @@ -95,8 +93,6 @@ func (d *Formatter) Render() (string, error) { return "", ErrNoTemplate } - log.Debug().Msgf("rendering template: %s", d.tmplString) - t := template.New("formattingTmpl").Funcs(funcMap()) t, err := t.Parse(d.tmplString) if err != nil { @@ -108,6 +104,10 @@ func (d *Formatter) Render() (string, error) { return "", fmt.Errorf("error while filling your template: %s", err.Error()) } + if buf.String() == "" { + return "", fmt.Errorf("template cannot be rendered, check your template") + } + return buf.String(), nil } diff --git a/pkg/formatting/formatter_test.go b/pkg/formatting/formatter_test.go index da11a6f..af41fbc 100644 --- a/pkg/formatting/formatter_test.go +++ b/pkg/formatting/formatter_test.go @@ -140,6 +140,16 @@ func Test_Render(t *testing.T) { assert.Error(err) assert.Contains(err.Error(), "error while filling your template: ") assert.Equal("", str) + + // Test with template with invalid format sended to a function + tmpl = New().WithTemplate(`{{ lookup "test" .Payload }}`).WithPayload([]byte(`{"test": "test"}`)) + assert.NotNil(tmpl) + assert.Equal(`{{ lookup "test" .Payload }}`, tmpl.tmplString) + + str, err = tmpl.Render() + assert.Error(err) + assert.Contains(err.Error(), "template cannot be rendered, check your template") + assert.Equal("", str) } func TestFromContext(t *testing.T) { diff --git a/pkg/formatting/functions.go b/pkg/formatting/functions.go index 99922ee..abc05ab 100644 --- a/pkg/formatting/functions.go +++ b/pkg/formatting/functions.go @@ -7,6 +7,7 @@ import ( "net/http" "reflect" "strconv" + "strings" "text/template" "time" @@ -26,6 +27,7 @@ func funcMap() template.FuncMap { "toPrettyJson": toPrettyJson, "fromJson": fromJson, "ternary": ternary, + "lookup": lookup, // Headers manipulation functions "getHeader": getHeader, @@ -153,14 +155,45 @@ func fromJson(v interface{}) map[string]interface{} { } // ternary returns `isTrue` if `condition` is true, otherwise returns `isFalse`. -func ternary(isTrue interface{}, isFalse interface{}, confition bool) interface{} { - if confition { +func ternary(isTrue interface{}, isFalse interface{}, condition bool) interface{} { + if condition { return isTrue } return isFalse } +// lookup recursively navigates through nested data structures based on a dot-separated path. +func lookup(path string, data interface{}) interface{} { + keys := strings.Split(path, ".") + + if path == "" { + return data + } + + // Navigate through the data for each key. + current := data + for _, key := range keys { + switch val := current.(type) { + case map[string]interface{}: + // If the current value is a map and the key exists, proceed to the next level. + if next, ok := val[key]; ok { + current = next + } else { + // Key not found + log.Logger.Warn().Str("path", path).Msg("Key are not found on the object") + return nil + } + default: + // If the current type is not a map or we've reached a non-navigable point + return nil + } + } + + // If the final value is a string, return it; otherwise + return current +} + // getHeader returns the value of the given header. If the header is not found, // it returns an empty string. func getHeader(name string, headers *http.Header) string { diff --git a/pkg/formatting/functions_test.go b/pkg/formatting/functions_test.go index c43b154..46fb8bb 100644 --- a/pkg/formatting/functions_test.go +++ b/pkg/formatting/functions_test.go @@ -114,6 +114,51 @@ func Test_ternary(t *testing.T) { assert.Equal("", getHeader("", nil)) } +func TestLookup(t *testing.T) { + // Initialize the assert helper + assert := assert.New(t) + + // Example of nested data structure for testing + testData := map[string]interface{}{ + "user": map[string]interface{}{ + "details": map[string]interface{}{ + "name": "John Doe", + "age": 30, + }, + "email": "john.doe@example.com", + }, + "empty": map[string]interface{}{}, + } + + // Test cases + tests := []struct { + path string + data interface{} + expected interface{} + }{ + // Test successful lookups + {"user.details.name", testData, "John Doe"}, + {"user.email", testData, "john.doe@example.com"}, + // Test unsuccessful lookups + {"user.details.phone", testData, nil}, + {"user.location.city", testData, nil}, + // Test edge cases + {"", testData, testData}, + {"user..name", testData, nil}, + {"nonexistent", testData, nil}, + // Test with non-map data + {"user", []interface{}{}, nil}, + } + + // Run test cases + for _, test := range tests { + t.Run(test.path, func(t *testing.T) { + result := lookup(test.path, test.data) + assert.Equal(test.expected, result, "Lookup should return the expected value.") + }) + } +} + func Test_getHeader(t *testing.T) { assert := assert.New(t) diff --git a/tests/integrations/options.js b/tests/integrations/options.js index 4fa56fc..5aedc21 100644 --- a/tests/integrations/options.js +++ b/tests/integrations/options.js @@ -17,9 +17,11 @@ export const session = (testName) => { } export const redisClient = new redis.Client({ - addrs: new Array(__ENV.REDIS_HOST+':6379'), + socket: { + host: __ENV.REDIS_HOST, + port: 6379, + }, password: __ENV.REDIS_PASSWORD, - db: 0, }); export const k6Options = { diff --git a/tests/integrations/scenarios.js b/tests/integrations/scenarios.js index 6e577d1..7112975 100644 --- a/tests/integrations/scenarios.js +++ b/tests/integrations/scenarios.js @@ -9,24 +9,37 @@ export const scenarios = [ name: 'basic-usage', description: 'should return 200 with the payload not formatted.', payload: { - message: `Hello, ${randomName}!`, + message: `Hello basic, ${randomName}!`, }, expected: { - message: `Hello, ${randomName}!`, - } + message: `Hello basic, ${randomName}!`, + }, + expectedResponse: '' }, { name: 'basic-formatted-usage', description: 'should return 200 with a basic formatting.', payload: { - message: `Hello, ${randomName}!`, + message: `Hello formatted, ${randomName}!`, }, expected: { "contentType": "application/json", data: { - message: `Hello, ${randomName}!`, + message: `Hello formatted, ${randomName}!`, } - } + }, + expectedResponse: '' + }, + { + name: 'basic-response', + description: 'should return 200 with a response asked.', + payload: { + id: randomName, + }, + expected: { + id: randomName, + }, + expectedResponse: randomName }, { name: 'advanced-formatted-usage', @@ -59,7 +72,8 @@ export const scenarios = [ childrenNames: ['Jane', 'Bob'], hasPets: false, favoriteColor: 'blue', - } + }, + expectedResponse: '' }, ] @@ -73,7 +87,9 @@ const testSuite = () => { const storedValue = await redisClient.lpop(`integration:${test.name}`); console.log(`[${test.name}]`, storedValue); + expect(JSON.parse(storedValue)).to.deep.equal(test.expected); + expect(res.body).to.equal(test.expectedResponse) }) }); } diff --git a/tests/integrations/webhooked_config.integration.yaml b/tests/integrations/webhooked_config.integration.yaml index 9708bdd..c9571c0 100644 --- a/tests/integrations/webhooked_config.integration.yaml +++ b/tests/integrations/webhooked_config.integration.yaml @@ -62,6 +62,37 @@ specs: # The key where you want to send the data key: integration:basic-formatted-usage +- name: basic-response + entrypointUrl: /integration/basic-response + response: + formatting: + templateString: '{{ fromJson .Payload | lookup "id" }}' + httpCode: 200 + security: + - header: + inputs: + - name: headerName + value: X-Token + - compare: + inputs: + - name: first + value: '{{ .Outputs.header.value }}' + - name: second + valueFrom: + staticRef: integration-test + storage: + - type: redis + specs: + host: + valueFrom: + envRef: REDIS_HOST + # Port of the Redis Server + port: '6379' + # In which database do you want to store your data + database: 0 + # The key where you want to send the data + key: integration:basic-response + - name: advanced-formatted-usage entrypointUrl: /integration/advanced-formatted-usage security: diff --git a/tests/webhooks.tests.yaml b/tests/webhooks.tests.yaml index 370725f..7c2271f 100644 --- a/tests/webhooks.tests.yaml +++ b/tests/webhooks.tests.yaml @@ -1,9 +1,13 @@ -apiVersion: v1alpha1 +apiVersion: v1alpha1_test observability: metricsEnabled: true specs: - name: exampleHook entrypointUrl: /webhooks/example + response: + formatting: + templateString: '{{ .Payload }}' + httpCode: 200 security: - header: id: secretHeader