coraza
coraza copied to clipboard
corazawaf
feat: adds tinygo support.
This PR adds proper support for tinygo.
HTTP server example has been turned into an own module to avoid those files to be considered in the tinygo test.
Depends on https://github.com/corazawaf/coraza/pull/296
I need to get this sorted first https://github.com/corazawaf/coraza/issues/283
Once https://github.com/corazawaf/coraza/pull/296 is merged we can get more greens.
Hi @jptosso - I am working with @jcchavezs on TinyGo support for coraza. I have managed to get it to work in https://github.com/jcchavezs/coraza/pull/1
Before updating this branch, I wanted to confirm one point - because TinyGo does not support encoding/json due to its use of reflection, the JSON test harness loading is switched to TinyJson and works fine. There is nothing similar for yaml though, while I see the testdata/engine test harnesses likely use yaml because it would be tedious maintaining the rules section without multi-line string support. For now I've converted the yaml to json using remarshal and check both in but wondering if this is reasonable or if you have any other preference. For example, would it be fine to migrate from yaml to just .go files with structs filled in? I don't think the maintenability suffers with such an approach.
Thanks!
@jptosso - I think we're almost ready with this. I have split out #307, would it help to further split out parts of this PR? Also, what do you think about converting the JSON test harnesses to Go, similar to #306? While tinyjson can generate code that works, perhaps it's unnecessary anyways.
Answering my own question, I guess it is probably not a good idea to convert the JSON to go. I found what seems to be the original source for the JSON files so we wouldn't want to diverge too much I think.
No problem, actually instead of generating some large tinyjson files, I'll rework to use gjson after #311.
@jptosso @jcchavezs Thanks for the patience, I think this is ready for a final review / merge
@jptosso Merged upstream and add some remaining fixes. Would be great if you can take a look at this as it'll be a significant milestone to Envoy support
Thanks @jptosso! Fixed the issue.
By the way, if you intended for auto-merge to work, not yet unfortunately
@anuraaga @jcchavezs do we have regression tests for tinygo to avoid breaking this milestone?
This is amazing milestone. Indeed we have the tinygo test command running on this which avoids regressions. Thanks @anuraaga
This is amazing milestone. Indeed we have the tinygo test command running
on this which avoids regressions
On Tue, 16 Aug 2022, 17:51 Juan Pablo Tosso, @.***> wrote:
@anuraaga https://github.com/anuraaga @jcchavezs https://github.com/jcchavezs do we have regression tests for tinygo to avoid breaking this milestone?
— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza/pull/254#issuecomment-1216826340, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAVGGD5EDZP5AWFULF3VZO2JTANCNFSM5XDUHDFA . You are receiving this because you were mentioned.Message ID: @.***>
I faced a similar situation with the testdata and was about to open an issue with the same concern. I think given the simplicity of the data that it is the same to have it in yaml or in go structs but having it in go makes them easier to maintain and removes this friction.
On Tue, 2 Aug 2022, 09:03 Anuraag Agrawal, @.***> wrote:
Hi @jptosso https://github.com/jptosso - I am working with @jcchavezs https://github.com/jcchavezs on TinyGo support for coraza. I have managed to get it to work in jcchavezs#1 https://github.com/jcchavezs/coraza/pull/1
Before updating this branch, I wanted to confirm one point - because TinyGo does not support encoding/json due to its use of reflection, the JSON test harness loading is switched to TinyJson and works fine. There is nothing similar for yaml though, while I see the testdata/engine test harnesses likely use yaml because it would be tedious maintaining the rules section without multi-line string support. For now I've converted the yaml to json using remarshal and check both in but wondering if this is reasonable or if you have any other preference. For example, would it be fine to migrate from yaml to just .go files with structs filled in? I don't think the maintenability suffers with such an approach.
Thanks!
— Reply to this email directly, view it on GitHub https://github.com/corazawaf/coraza/pull/254#issuecomment-1202097597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAQYZKN2GES2XN766HTVXDB2TANCNFSM5XDUHDFA . You are receiving this because you were mentioned.Message ID: @.***>