Skip to content
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

Sieve: expression negation, nested if / mixed conditionals and actions, support for non-strings in add/add!/update #1923

Merged
5 commits merged into from
May 14, 2021

Conversation

monoidic
Copy link
Contributor

@monoidic monoidic commented May 1, 2021

This PR has three parts:

  • generic negation of arbitrary expressions, along with the removal of the list match negation
  • nested if statements, plus mixed actions and actions in the same scope
  • the attribute manipulation actions add, add! and update support non-string (bool/int/float) values

There is also an RFC-ish last commit, with some somewhat breaking changes.

  • Some operators which were made redundant by negation (!~ and :notcontains) are removed
  • :in is used for list inclusion checks instead of ==
  • there's a clear split between single-value and multi-value matches, with a couple of new operators due to that: :containsany and :regexin for string matching against multiple regexes or string inclusions)

The last commit is not expected to be accepted at the moment. It'll break a bunch of stuff. Though comments on it would be appreciated.

@codecov-commenter
Copy link

codecov-commenter commented May 1, 2021

Codecov Report

Merging #1923 (2c75a9d) into develop (a7ecf62) will increase coverage by 0.40%.
The diff coverage is 99.48%.

@@             Coverage Diff             @@
##           develop    #1923      +/-   ##
===========================================
+ Coverage    75.15%   75.56%   +0.40%     
===========================================
  Files          400      406       +6     
  Lines        21516    21937     +421     
  Branches      2874     2924      +50     
===========================================
+ Hits         16170    16576     +406     
- Misses        4674     4679       +5     
- Partials       672      682      +10     
Impacted Files Coverage Δ
intelmq/bots/experts/sieve/expert.py 82.63% <93.75%> (+0.69%) ⬆️
intelmq/tests/bots/experts/sieve/test_expert.py 100.00% <100.00%> (ø)
intelmq/bots/experts/rdap/expert.py 68.86% <0.00%> (-3.41%) ⬇️
intelmq/bots/experts/maxmind_geoip/expert.py 21.48% <0.00%> (-0.65%) ⬇️
intelmq/bots/experts/domain_suffix/expert.py 44.70% <0.00%> (-0.65%) ⬇️
intelmq/bots/experts/tor_nodes/expert.py 48.75% <0.00%> (-0.64%) ⬇️
...telmq/bots/experts/recordedfuture_iprisk/expert.py 43.82% <0.00%> (-0.63%) ⬇️
intelmq/tests/lib/test_utils.py 99.43% <0.00%> (-0.57%) ⬇️
intelmq/bots/experts/asn_lookup/expert.py 37.71% <0.00%> (-0.55%) ⬇️
intelmq/bin/intelmqctl.py 12.88% <0.00%> (-0.06%) ⬇️
... and 13 more

@ghost
Copy link

ghost commented May 3, 2021

Wow, great work! I'm just missing the docs :D

The last commit is not expected to be accepted at the moment. It'll break a bunch of stuff. Though comments on it would be appreciated.

I have no problems with breaking changes

  • as long as they are well documented
  • in major release (3.0 is ahead, so that's OK)
  • if behaviour doesn't change in unexpected ways, in this case the behaviour of == with lists is such a candidate. However, I'm open for discussions on this item =)

@monoidic
Copy link
Contributor Author

Some small adjustments, like dropping the negation tests on list/set actions. I will add the breaking changes in a seperate PR, this is enough for one PR as is.

@monoidic monoidic changed the title Sieve: expression negation, nested if / mixed conditionals and actions, support for non-strings in add/!add/update Sieve: expression negation, nested if / mixed conditionals and actions, support for non-strings in add/add!/update May 12, 2021
@monoidic
Copy link
Contributor Author

@wagner-certat Thoughts?

@ghost ghost added this to the 3.0.0 milestone May 14, 2021
@ghost ghost added the component: bots label May 14, 2021
@ghost ghost self-assigned this May 14, 2021
@ghost
Copy link

ghost commented May 14, 2021

Some small adjustments, like dropping the negation tests on list/set actions. I will add the breaking changes in a seperate PR, this is enough for one PR as is.

Thank you very much - again!

@wagner-certat Thoughts?

We had public holidays yesterday (and sometimes I may be busy :) )

@ghost ghost merged commit 3944744 into certtools:develop May 14, 2021
@ghost
Copy link

ghost commented May 14, 2021

changelog entry added in 7ca54fb

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants