-
Notifications
You must be signed in to change notification settings - Fork 18
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
Replace criteo/haproxy-spoe-go with negasus/haproxy-spoe-go #51
Comments
Sounds reasonable to me. |
Up for a PR? @mac-chaffee |
Still a work a progress. It's turning out to be a bigger lift than expected, but I think it'll be worth it |
@mac-chaffee |
Hey all, just to add to @15ljindal message we also looking into to create a spoe implementation for Crowdsec. Saw the lib you was using and that the question arose of the maintainer. Any feedback you can give will help. Thank you for the awesome work! |
I hit a wall with implementing this issue, mostly due to my own weak Golang skills but also due to other issues in this repo that might make sense to tackle in parallel (like config management, logging, and graceful config loading, which all touch the same pieces of code that this issue would touch). Also been busy at my job where our project to migrate to coraza has been pushed down the priority list in favor of more pressing things, so sorry about that! I believe @sts had been considering looking into this issue. About the two libraries, the criteo one is definitely rougher around the edges. The public interface is quite sparse, test coverage isn't great, and there is essentially no way to construct a SPOE message without having a raw byte array, which makes testing difficult. The negasus one appears slightly better in every way (albeit also not having recent activity). One problem I was hitting was that parsing SPOE messages requires a clever use of the type system. The negasus library spits out Which is not much better than what we have now with the criteo library: coraza-spoa/internal/request.go Line 71 in d615d83
Maybe a more skilled golang user could find a better solution. PS: I don't mean to denigrate either criteo or negasus. I greatly appreciate both your work! |
Because of all these different approaches with either different api quality and/or speed, I reimplemented it with a zeroalloc hotpath. I will add some examples for e.g. a L7 Client validation and probably move it to a different Github Org soonish. If there are any requests for changes/additions feel free to ping me. Its still not versioned or has a stable API as I first want to have some examples and tests added but it does already work fairly well |
@fionera thanks for coming by. My 2p on this matter, not as a coraza maintainer but more like an open source person is that a good way to get your library right to exercise the API with a good use case that verify the assumptions in your abstractions. Coraza is indeed a good use case and if you wanna give it a try to a coraza connector using your library we will be more than happy to help review and answer doubts. I also opened a couple of issues in your repo with general practices. |
I think I will give it a try and make a PR porting coraza-spoa to my library :) |
We seem to have encountered a memory leak with haproxy-spoe-go. Managed to reference the wrong lib.. negasus/haproxy-spoe-go#18 Any updates on replacing criteo/haproxy-spoe-go with negasus/haproxy-spoe-go? The criteo/haproxy-spoe-go project seems less active in comparison. |
As reference: #103 |
The library that we use to speak SPOP appears to be unmaintained: criteo/haproxy-spoe-go#37
It also has a small public API and lack of interfaces that makes testing our code extremely difficult. For example, there's no way to construct our own spoe.Message in Go; you need to give it a real byte array. I hit this issue when trying to add some unit tests to coraza-spoa.
There is another more-active library Stefan found that I think we should use: https://github.com/negasus/haproxy-spoe-go
I'm about 50% done with a proof of concept PR to complete this. Just thought I'd file a tracking ticket.
The text was updated successfully, but these errors were encountered: