poolpOrg
RFC: opensmtpd-filters-go library and filter port
Hi Gilles,
over the last couple of months I worked on a Go library for implementing opensmtpd filters (opensmtpd-filters-go. It's somewhat based around the code of filter-rspamd, but focused on making implementation as easy as possible by wrapping each event sent by opensmtpd into an interface.
Also, as far as I can tell, it seems that calling Print* from goroutinesnot thread-safe appears to be (if you can what I did theseere). So I added a SafePrintf|ln implementation that uses channels to ensure that messages aren't picked apart by the kernel during concurrent operations.
I also started porting filter-rspamd as a proof of concept and wrote dnsbl, trace and greylist implementations. All of this is pretty much under active development, but I'd be interested in your thoughts.
And a happy new year to you :).
Cheers, Jonas
I like the overall idea. It makes the code base slightly more complex but reduces code duplication quite a bit. In the past, I had to (and will have to) resubmit many patches I submitted to filter-senderscore to the other filters. Most of that would no longer be needed with a library. We should let Gilles decide whether it's worth introducing the extra layer, though.
Before it can be used for the currently existing filters, some important features are missing, such as support for legacy protocol versions or a test mode to process the commands fully sequentially, see e.g. https://github.com/poolpOrg/filter-senderscore/blob/109a582/filter-senderscore.go#L172
I like the idea of a library and I had on my plans to write one but was waiting for the API to stabilise a bit before diving into it. I'm convinced that a library to abstract the protocol between OpenSMTPD and the filter is the way to go, but I'm not too convinced by the interface you propose as I think it goes too far and makes the filter code itself less understandable by handling tasks behind the scene.
This is maybe a matter of taste but to me by handling some things behind the scene, like collecting the sender and rcpt addresses, you make the filter code smaller but more confusing, I can no longer know what happens by reading my filter code alone, I have to go figure what your library does.
In my opinion, the library should provide just the right level of abstraction, which is hiding the stdin/stout logic, allowing a filter to register callbacks for certain events and triggering them, but the filter should be in charge of everything else. If there are some things that are common between multiple filters, they could be provided as helpers, but ultimately it should be the filter that holds the logic. Not seeing the sender address or recipient address collecting code from my filter is very confusing to me.
Again this is a matter of taste, some people might prefer writing smaller filters and not worry about this, but I personally feel uncomfortable with that. Have you seen the libraries I had written for C and Python ?
@poolpOrg
This is maybe a matter of taste but to me by handling some things behind the scene, like collecting the sender and rcpt addresses, you make the filter code smaller but more confusing, I can no longer know what happens by reading my filter code alone, I have to go figure what your library does.
In my opinion, the library should provide just the right level of abstraction, which is hiding the stdin/stout logic, allowing a filter to register callbacks for certain events and triggering them, but the filter should be in charge of everything else. If there are some things that are common between multiple filters, they could be provided as helpers, but ultimately it should be the filter that holds the logic. Not seeing the sender address or recipient address collecting code from my filter is very confusing to me.
I agree. I went back and forth between having either only the filter/reporting events or also abstract away the rcpt addresses etc. After iterating over a lot of different API implementations for the library I settled for the current approach of interfaces because of that issue. In a word, the interface approach enables composition. So all of that "extended functionality", i.e. anything that goes beyond routing the events, I moved into an optional mixin SessionTrackerMixin. I feel ilke that's a really good balance. You can easily implement everything yourself, keeping all the code in your filter and choosing the ideal data structure, or you can compose commonly occurring functionality into your filter. I feel like that's a good Golang-native way for providing such "helpers", as you called them.
Again this is a matter of taste, some people might prefer writing smaller filters and not worry about this, but I personally feel uncomfortable with that. Have you seen the libraries I had written for C and Python ?
Python I looked at... C I actually am not aware of. The approach I've chosen in the end is very Golang specific, but I iterated over multiple different implementations while developing the library. Registering individual callbacks with the library was one of them, but it leads to very strange code in Golang as it's hard to pass bound callbacks correctly for the caller, which led to the current "interface discovery" approach.
This won't be moving forward as I don't have intent to refactor the filter, just maintain it