Spike solution for filtering tests by job group

[kraih]

Instead of regexes i've used globs, which are a little more user friendly and less prone to security issues. The implementation is a bit inefficient, since it loads all job groups into memory to filter them on the Perl side. But there will probably never be an openQA instance with enough job groups to justify the complexity of filtering in SQL.

Since this is a spike solution, tests and ui enhancements have been left out. The feature can only be used with query parameters in the URL for now, like /tests/overview?group_glob=*kernel*&not_group_glob=*development*&todo=1. Multiple group_glob and not_group_glob values can be used for more complex filtering.

Progress: https://progress.opensuse.org/issues/119746

[Martchus]

Looks generally good (for a spike solution, to merge tests would be good).

[Martchus]

Maybe avoid the double map calls (which in Perl likely result in an intermediate array, right?):

Suggested change

	my @inclusive = map { qr/$_/i } map { glob_to_regex_string($_) } @$group_glob;
	my @inclusive = map { my $regex = glob_to_regex_string($_); qr/$regex/i } @$group_glob;

[kraih]

Too cheap to worry about resource usage, the intermediate list costs nothing really. Most readable solution wins here imo.

[okurz]

I would leave out the "_glob" suffix.

[kraih]

That would also have been my preference originally too, but we've already set a precedent here with module_re for regex. Feels kinda wrong now not to have a special suffix for globs too.

[okurz]

I see. As we already have regex for module then can we please reconsider to use group_re with regex here as well? I don't see the security risk as we merely match on job group names, isn't it?

[kraih]

Perl regexes are very powerful and compiling arbitrary strings from the internet into them has a lot of room for exploitation. For example i could probably use lookahead/behind to create a DoS attack even with small input strings. 🤔 Using globs would be a lot safer and easier to understand by users.

[Martchus]

Yes, unfortunately Perl has no good way of dealing with regexes from users. Unfortunately we already do this. I once wrote the regex_match helper so we at least don't get warnings in our server logs but a DoS attach is likely possible. So I guess using globbing is better here, indeed.

(We can actually always make use of native code and call a library that is more limited. However, that's more work.)

[kraih]

For more information see also https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS. The current Perl regex engine is actually better than most at dealing with such cases, but it still can happen.

[okurz]

I wonder about the example that you mention about /tests/overview. https://progress.opensuse.org/issues/119746 is about /tests. Does this also work for /tests or was that aspect overlooked?

[okurz]

I see. As we already have regex for module then can we please reconsider to use group_re with regex here as well? I don't see the security risk as we merely match on job group names, isn't it?

[kraih]

I wonder about the example that you mention about /tests/overview. https://progress.opensuse.org/issues/119746 is about /tests. Does this also work for /tests or was that aspect overlooked?

Yes, that aspect was indeed overlooked after the discussions about the feature. Do we really want the group filtering in the /tests context rather than /tests/overview? I didn't even think about /tests because it fits so well into /tests/overview. Unfortunately the code paths are completely separate and probably won't be able to share much.

[kraih]

Yes, that aspect was indeed overlooked after the discussions about the feature. Do we really want the group filtering in the /tests context rather than /tests/overview?

As discussed on Slack, there is interest in having both. So i will look into extending the spike solution.

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏

[kraih]

Followup PR has been merged.