Breaking: restrict unintended user constant usage

[kares]

This change effectively restricts users shooting themselves in the foot with code => ... such as :

foo = event.get('foo')
NP = event.get('NP') if event.get('NP')
if NP != nil # constant value carried over to next event
  event.set('found', NP)
end

In filter version <= 3.1.5 this piece executes with warnings about already initialized constant NP but users often ignore these as they're not familiar with Ruby effectively leading to "unexpected" interference between events.

The change to use normal def self.method opposed to define_method { ... } with block, effectively prevents dynamic constant assignments as its not allowed

This is a breaking change - invalid code => ... will break early on (failing to start LS).
The trade-off for users to review such pieces and potentially avoid "weird" events is acceptable.

• also changed to log exception and backtrace on Ruby raises that are getting tagged

[yaauie]

+1.

As a breaking change, we will need to guard against accidental inclusion in Logstash 7.x distributions. Preferably, we would also include a non-breaking release of this gem for inclusion in 7.x that used the deprecation logger to warn off users who exploit the current implementation.

• Can we similarly guard against setting global variables?
• Should we guard against instance variables and class variables (or require a user opt-in to make them accessible)?

[kares]

Preferably, we would also include a non-breaking release of this gem for inclusion in 7.x that used the deprecation logger to warn off users who exploit the current implementation.

Not sure we could do that with confidence, we could take a snapshot of Object constants before and after but can never know for sure the changes are only due to the executing script.

Could try detecting Capital = ... using a regexp and warn if that makes sense as a middle step.
(as I do not see an added benefit of parsing the script using Ripper)

Can we similarly guard against setting global variables?

Dynamic constant assignment in a method is a parsing feature, of course only handles Some = ... but one can still force a constant with const_set.

Assigning global variables is always allowed so in this case same 2 options: detect using regexp or use Ripper and walk the returned expression tree - detecting direct $var assignments.

Should we guard against instance variables and class variables (or require a user opt-in to make them accessible)?

Not sure as it will always have limitations.
IMO, binding self to be a dummy object instead of plugin instance could improve things.

If we decide to take it all the way through a deprecation phase than its worth considering more "breaks", personally would at least decouple scripts from plugin instance, as its relative easy to run into some of the state there.

[kares]

changed the work here to be a major and also included another backwards compatibility "breaking fix" for #60
(to be merged/released before LS 8.0 is in the works)