Exposing some of the hidden packages behind internal folder.
#1094
Comments
|
Hey! The reason behind this is long-term maintenance; if you want a specific API or set of APIs, please mention them on this issue so we can review this. We don't have any problem with exposing additional APIs as long as it's safe, it doesn't break compatibility, and there is a clear use case. There are certain limitations, for example, we cannot expose additional functions for existing immutable interfaces, as it breaks compatibility. In which case we would have to release Wrappers to access to them. Like: tx := tx.(types.TransactionState)
tx = tx.(otherpackage.TransactionExtended) |
|
In my case described above I guess I would want access to:
|
|
Do you want to create a new parser, or just add some directives? |
|
No, some utility that checks rules validity and has a bit more verbose error messages in regards to where the actual broken rule is (file:linenumber). |
|
Thanks for raising this @s3rj1k. There are a few reasons why we don't
expose the parser and one of them is the API. The parser being internal
allows us to accommodate the API aiming the best for the WAF, exporting it
means we need to maintain compatibility and settle an API. I propose you
the following: We can export the parser through the experimental API and
then you can consume it and propose improvements when it comes to erroring
and UX.
…On Fri, Jul 12, 2024 at 11:02 PM s3rj1k ***@***.***> wrote:
No, some utility that checks rules validity and has a bit more verbose
error messages in records to where the actual broken rule is
(file:linenumber).
—
Reply to this email directly, view it on GitHub
<#1094 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXOYAWZRT66I3BDRQ2257LZMA76RAVCNFSM6AAAAABKWX5VGSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRWGM2DQNZZGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
@jptosso Another solution maybe just standalone CLI utility in repository that just loads ruleset and reports error if any, I am fine with doing PRs as long as there is some kind of simplified entry point for rule testing that is. Also this will be similar to what has libmodsecurity, they ship standalone CLI to just do rule verification. This way there is no need to wraparound into experimental API. |
|
This way there is no need to wraparound into experimental API.
experimental API isn't a workaround, it is the official way for us to
expose APIs we don't currently expose. Providing a CLI is not something we
are inclined to for a couple of reasons: 1. you are the first person asking
for it and 2. It is a maintenance burden for us (volunteering project). How
about we expose the parser through an experimental API which we reshape if
needed and you maintain the CLI and if the CLI becomes stable enough we
could think to foster it under coraza? Does that make sense?
…On Mon, Jul 15, 2024 at 11:50 PM s3rj1k ***@***.***> wrote:
@jptosso <https://github.com/jptosso> Another solution maybe just
standalone CLI utility in repository that just loads ruleset and reports
error if any, I am fine with doing PRs as long as there is some kind of
simplified entry point for rule testing that is.
Also this will be similar to what has libmodsecurity, the also ship
standalone CLI to just do rule verification.
This way there is no need to wraparound into experimental API.
—
Reply to this email directly, view it on GitHub
<#1094 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXOYAWQAZVWNRV7L47VGTDZMQ7ZBAVCNFSM6AAAAABKWX5VGSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRZGQ4TQNJUGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
@jptosso Yea, sounds also good |
|
+1 for exposing the parser. Use cases:
|
|
I am inclined to export the parse but I would expect to have implementation code using this. |
Do you mean like the example in the PR that you referred to? BTW, the current proposal in the PR doesn't expose the rules themselves, but only a parsing error if it occurred. Exposing the rules is beneficial for users that build ruleset management systems. |
|
@guydc You can just rsync the whole thing really, I don't understand all the hesitation of upstream devs about moving packages out of |
Coraza is a volunteering project and more public packages mean more maintenance, more bugs and more constraints when refactoring internals hence the hesitation. I have worked for 9 years in OSS now and if I would get a dollar every time someone said "export this internal because I need it" and then that person never came back or showed how the big refactor benefit them I would probably be able to have a nice trip to Disneyland Paris this April. Finally I learnt from a OSS master long ago that user must "request for the feature, not for the implementation". Do you need certain functionality? bring in the use case, show us code where the API is lacking and we can discuss how to fix it, requesting a package to be exported is a not feature but an implementation detail. Do you need for us to expose the rules? fine, show us what do you need and what would be the information you need. We don't have to expose internal objects for that, we can build a new abstraction that will work for you keeping the internals internal. You can even come up with a draft PR instead that proves value so we can support the use case. Would you spend your volunteering time reworking and API (and the future maintenance of it) just by trusting me saying "I need it"? Don't take me wrong, I love users requesting for features but as you could imagine we would not spend our own time working for a user desire but instead a use case that can benefit the community. So, show us the use case and we will happily lead the feature to completion. |
You can handle that by stating in docs that no support is given and no compatibility promises for modules in that list. |
My use case was this #1094 (comment) |
|
@s3rj1k you asked for a parser which would validate rules and return errors with line number and we proposed #1101 then you said you had no time to show any code consuming this parser (#1101 (comment)). Didn't we aid you with your use case? I spent time crafting the PR and you haven't show any usage on that so what is the ROI for the project? Now @guydc is asking for exposing the rules (I assume given a string input give me the list of rules) that is a different use case (very valid IMHO) that we can also address in an experimental feature and we could even come up with such a PR but the request will be the same, with the experimental feature show us some usage code. |
That should be read like "Project is under NDA" |
|
Ok, I see no point in continuing this discussion, @guydc feel free to openup new issue if you are up for the challenge. Closing this as I no longer work on related project. |
|
Hi @jcchavezs, @s3rj1k, Thanks for taking my interest under consideration. The use case I had in mind for exposing the rules is to be able to allow end-users to interact with rules from a large ruleset like CRS in a user-friendly manner. For example, one could build a UI/API that allows users to:
A lot of WAF vendors offer similar capabilities. I imagine that this can help drive adoption by simplifying operations. To achieve this, rules do not have to be entirely exposed. Some sort of limited metadata is probably sufficient. It's also possible that this sort of functionality can just be a part of Coraza that relies on internals without exposing the rules. I'll open a follow-up issue. |
Exactly, libmodsecurity has CLI utility for interacting with ruleset that gives some usable UX to work with large amount of rules. |
I think that this
internalfolder that hides most of exported modules is an unfortunate situation.Consider a use-case when someone wants to make a rules parser to help migrate off
libmodsecurity, in current state this can only be done by actually launchingwafviaNewWAFfunction, this works but is a bit uncomfortable.Furthermore, considering the case above, when broken rule is encountered, in some cases, error message is so obscured with error wrapping and lacks the actual information that can help to find the actual broken rule as opposed to how
libmodsecurityreports parse errors (with partial rule and file:linenumber).By exposing so of the
internalpackages to public it will allow to reuse them more comfortably and will help improve error messaging, at least there will be no multiple/useless error wrapping for parser code.The text was updated successfully, but these errors were encountered: