Compatibility with Graylog v5

frantz45 commented 1 year ago

Graylog v5.0 has been released. We should release a compatible version of our plugins for this new version.

c8y3 commented 1 year ago

relation to https://github.com/airbus-cyber/graylog-plugin-correlation-count/issues/30

c8y3 commented 1 year ago

Things to do during the migration:

remove all refs. In particular when using the graylog BootstrapModalForm component
introduce .tsx
convert components into functional form

c8y3 commented 1 year ago

Do not forget to update plugins' version number major digit to 5.

c8y3 commented 1 year ago

Do not forget to update the compatibility matrix in the README.

c8y3 commented 1 year ago

Must update:

mongo version to 5 (docker-compose)
java version to 17 (must create a new build docker)

frantz45 commented 1 year ago

Must update:

mongo version to 5 (docker-compose)

java version to 17 (must create a new build docker)

I don't know about the docker version, but now Graylog includes Java so you don't need to install it. I recommend MongoDB v6 instead of v5.

c8y3 commented 1 year ago

To compute the information displayed in column Alerts of the alert rules list (/wizard/AlertRules), the plugin relied on some Graylog code which was used for the deprecated legacy alerts APIs. With Graylog 5.0, these APIs and underlying code have been removed (https://github.com/Graylog2/graylog2-server/blob/5.0/UPGRADING.md#legacy-alert-api).

From my understanding of the previous code, the information displayed in column Alerts were:

the average number of messages incoming per day in the underlying stream of the alert since the last modification of the alert
the total number total number of messages in the underlying stream of the alert since the last modification of the alert To me, this column was not correctly labeled, it should have rather been Messages (and then messages/day). It seems similar to what is displayed in the stream list (/streams) as messages/second.

I am unsure what would be the best line of action to migrate this code. Here are some options:

remove column Alerts since it is misleading and clean up code (simplest)
try to mimic what's being done in the stream list and change the labels (medium difficulty, I am not sure we can get the total number of messages, or the average per day, I have to study the code more)
try to really compute and display the number of alerts triggered for this rule and the average per day (hardest)
remove column Alerts for the time being but create another evolution issue to get the functionality back later

Please, let me know what would be best. Also, if you know of any parts of Graylog that already display the number of alerts triggered for an event definition, tell me. As it would be a good entry point to study.

Additional note: we should think about the case when alert rules have two streams and two event definitions. The previous implementation was probably already handling this case incorrectly.

frantz45 commented 1 year ago

To compute the information displayed in column Alerts of the alert rules list (/wizard/AlertRules), the plugin relied on some Graylog code which was used for the deprecated legacy alerts APIs. With Graylog 5.0, these APIs and underlying code have been removed (https://github.com/Graylog2/graylog2-server/blob/5.0/UPGRADING.md#legacy-alert-api).

From my understanding of the previous code, the information displayed in column Alerts were:

the average number of messages incoming per day in the underlying stream of the alert since the last modification of the alert

the total number total number of messages in the underlying stream of the alert since the last modification of the alert To me, this column was not correctly labeled, it should have rather been Messages (and then messages/day). It seems similar to what is displayed in the stream list (/streams) as messages/second.

I am unsure what would be the best line of action to migrate this code. Here are some options:

remove column Alerts since it is misleading and clean up code (simplest)

try to mimic what's being done in the stream list and change the labels (medium difficulty, I am not sure we can get the total number of messages, or the average per day, I have to study the code more)

try to really compute and display the number of alerts triggered for this rule and the average per day (hardest)

remove column Alerts for the time being but create another evolution issue to get the functionality back later

Please, let me know what would be best. Also, if you know of any parts of Graylog that already display the number of alerts triggered for an event definition, tell me. As it would be a good entry point to study.

Additional note: we should think about the case when alert rules have two streams and two event definitions. The previous implementation was probably already handling this case incorrectly.

IMHO this column can be removed

c8y3 commented 12 months ago

The choice of the user to display the Alert column is registered in the configuration. In theory removing this configuration should not pose any problem, but we should still check there aren't any problem when migrating from a 4.5.0.

c8y3 commented 12 months ago

As indicated in the changelog, endpoint plugins/com.airbus_cyber_security.graylog.wizard/alerts/data will be removed in 5.0.0. Use plugins/com.airbus_cyber_security.graylog.wizard/alerts instead.

c8y3 commented 10 months ago

Migration to graylog 5.1 proved to be more difficult than expected. It seems I eventually solved the issues. Mainly:

end-to-end tests were blocking forever because graylog was stopping at startup during disk sanity checks (see https://github.com/airbus-cyber/graylog-plugin-alert-wizard/actions/runs/5949993932/job/16136902705). The tests were waiting forever for startup to complete. I have now set a timeout and all docker volumes are now cleaned up between each steps to regain disk space. They were accumulating to the point of saturating all disk space of the test environment.
Graylog javascript component to input fields with typeahead (https://github.com/Graylog2/graylog2-server/blob/5.1.3/graylog2-web-interface/src/components/common/TypeAheadFieldInput.jsx) seems to have a bug when it is used in the context a react functional component. When the onChange method is called it doesn't have access to the most up to date value of the component's state. I really do not understand why, but I found a way to wrap the component to circumvent the bug. I hope it works in all cases. Ideally, I should open an issue in graylog, but haven't had the time to reproduce with a standalone example.

frantz45 commented 10 months ago

Well done for the 5.1.0 release :) But you forgot to update the compatibility array in the README.md. No need to release a new version, a push in the master branch is enough

frantz45 commented 10 months ago

I've tested with Graylog v5.1.4 and I get some issues.

I get a crash on the rules list page:

I don't know exactly the trigger but it happens very often after some seconds when moving the mouse over the rules and sometimes by clicking multiple times on rules. It's not a blocking issue because you can access the list again by reloading the page.

Full stack trace:

Minified React error #130; visit https://reactjs.org/docs/error-decoder.html?invariant=130&args[]=undefined&args[]= for the full message or use the non-minified dev environment for full errors and additional helpful warnings.

Stack Trace:

wi@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:14363
4448/ui/<@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:48506
en@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:58729
po@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:63097
qt@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:62783
Rs@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:9419
ys@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:893
Xs@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:816
Gi@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:87282
4448/oe/<@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:37228
4203/l.unstable_runWithPriority@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:681:4034
ir@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:36938
oe@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:37176
sr@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:37109
J0@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:18863
R0@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:671:2219
mo@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:15170
Kn@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:14726
4203/l.unstable_runWithPriority@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:681:4034
Jn@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:14596

Component Stack:

    in Position
    in Transition
    in RootCloseWrapper
    in Portal
    in w
    in div
    in styled.div
    in y
    in Sn
    in td
    in tr
    in Xs
    in tbody
    in table
    in styled.table
    in div
    in div
    in div
    in div
    in Qs
    in div
    in AlertRuleList
    in injectIntl(AlertRuleList)
    in pn
    in div
    in w
    in div
    in y
    in Styled(y)
    in div
    in Va
    in IntlProvider
    in vh
    in m
    in i1
    in _1
    in div
    in I
    in Styled(I)
    in l
    in div
    in styled.div
    in O
    in i1
    in _1
    in div
    in styled.div
    in Ko
    in Vs
    in div
    in styled.div
    in i
    in Ge
    in t
    in _t
    in Qn
    in _
    in i1
    in R0
    in E1
    in m1
    in m0
    in Q
    in ge
    in We
    in ConnectStoresWrapper[We] stores=streams
    in qe
    in le
    in k
    in nt
    in ot
    in Ye
    in at
    in st
    in dt
    in M
    in Pt
    in _e
    in Suspense
    in f
    in Unknown
    in X
    in le
    in k
    in i
    in $d
    in at

frantz45 commented 10 months ago

I get another crash:

It's because in the database there is the old "Alerts" column in the Wizard configuration:

But I don't know how it happens because before installing the new plugin there was no Wizard configuration in the database. I don't manage to reproduce it systematically but it happens multiple times. When it happens once then I can't access the Wizard, I need to remove the Wizard configuration in the database. It makes me think in a case of an update, do I need to remove the Wizard configuration in the database to avoid this ? Or could you handle it in the code ?

Full stack trace:

this._availableFieldName().filter(...)[0] is undefined

Stack Trace:

_getFieldName@https://th8-sem3.sso.spc3.int/assets/plugin/com.airbus-cyber-security.graylog.AlertWizardPlugin/plugin.com.airbus_cyber_security.graylog.AlertWizardPlugin.5f8b465f41b36511db9a.js:2305:15271
UlRDpKPf/_sortableItems/<@https://th8-sem3.sso.spc3.int/assets/plugin/com.airbus-cyber-security.graylog.AlertWizardPlugin/plugin.com.airbus_cyber_security.graylog.AlertWizardPlugin.5f8b465f41b36511db9a.js:2305:15525
_sortableItems@https://th8-sem3.sso.spc3.int/assets/plugin/com.airbus-cyber-security.graylog.AlertWizardPlugin/plugin.com.airbus_cyber_security.graylog.AlertWizardPlugin.5f8b465f41b36511db9a.js:2305:15495
render@https://th8-sem3.sso.spc3.int/assets/plugin/com.airbus-cyber-security.graylog.AlertWizardPlugin/plugin.com.airbus_cyber_security.graylog.AlertWizardPlugin.5f8b465f41b36511db9a.js:2305:19185
po@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:62996
qt@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:62783
Rs@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:9419
ys@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:893
Xs@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:674:816
Gi@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:87282
4448/oe/<@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:37228
4203/l.unstable_runWithPriority@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:681:4034
ir@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:36938
oe@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:37176
sr@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:37109
Ta@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:84027
K0@https://th8-sem3.sso.spc3.int/assets/vendor.8a712d83b311029bb0b6.js:672:55219
UlRDpKPf/vh/</<@https://th8-sem3.sso.spc3.int/assets/plugin/com.airbus-cyber-security.graylog.AlertWizardPlugin/plugin.com.airbus_cyber_security.graylog.AlertWizardPlugin.5f8b465f41b36511db9a.js:2305:37034

Component Stack:

    in ManageSettings
    in pn
    in span
    in p
    in styled.p
    in div
    in styled.div
    in div
    in styled.div
    in div
    in w
    in div
    in y
    in Styled(Styled(y))
    in Ws
    in Pn
    in div
    in Va
    in IntlProvider
    in vh
    in m
    in i1
    in _1
    in div
    in I
    in Styled(I)
    in l
    in div
    in styled.div
    in O
    in i1
    in _1
    in div
    in styled.div
    in Ko
    in Vs
    in div
    in styled.div
    in i
    in Ge
    in t
    in _t
    in Qn
    in _
    in i1
    in R0
    in E1
    in m1
    in m0
    in Q
    in ge
    in We
    in ConnectStoresWrapper[We] stores=streams
    in qe
    in le
    in k
    in nt
    in ot
    in Ye
    in at
    in st
    in dt
    in M
    in Pt
    in _e
    in Suspense
    in f
    in Unknown
    in X
    in le
    in k
    in i
    in $d
    in at

c8y3 commented 10 months ago

Just passing by... Sorry for the regressions. For the first problem, it seems it happens when the mouse hovers over the User column. In the code source there is a spurious line break in the middle of the tooltip element (https://github.com/airbus-cyber/graylog-plugin-alert-wizard/blob/5.1.0/src/web/wizard/components/rules/AlertRuleList.jsx#L278). Maybe I fell asleep on my keyboard at some point :) But this doesn't seem to be the cause of the problem :( Also we are using OverlayElement (https://github.com/airbus-cyber/graylog-plugin-alert-wizard/blob/5.1.0/src/web/wizard/components/rules/AlertRuleList.jsx#L302) which is not used anymore in all of graylog interface. So I wonder if the component still work and if we should not better opt for another component. Do you remember how the tooltip was rendering? Do you have an example in graylog of a similar rendering which I could mimic? Thank you.

frantz45 commented 10 months ago

Another regression:

Create a rule
Add a condition with a list. For example "user" "is present" in list "test"

When you select the list "test" among other lists, it will take the place of the condition "is present", and the last field representing the choosen list will be empty. Then you can't save the rule because the Save button is grey. If you type a value in the blank field you can save the rule but an error is displayed.

Note: when this issue will be fixed, try the "Test" button because it doesn't seem to work in previous versions

c8y3 commented 10 months ago

The stack-trace when hovering over the user name turned out to be an incorrect import.

c8y3 commented 9 months ago

About the old "Alerts" column present in the database, I am unable to find the culprit in the code base: the only way I was able to reproduce is by a call to the REST API (a PUT to endpoint /api/plugins/com.airbus_cyber_security.graylog.wizard/config). Maybe there are some scripts doing just that behind your back? Also, in the case of an upgrade, there should be a migration of the configuration, this can be done with the REST API. I am adding a small note in the changelog and readme about this.

frantz45 commented 9 months ago

The stack-trace when hovering over the user name turned out to be an incorrect import.

I confirm it's fixed in the latest master build (v5.1.1)

frantz45 commented 9 months ago

Another regression:

Create a rule

Add a condition with a list. For example "user" "is present" in list "test"

When you select the list "test" among other lists, it will take the place of the condition "is present", and the last field representing the choosen list will be empty. Then you can't save the rule because the Save button is grey. If you type a value in the blank field you can save the rule but an error is displayed.

Note: when this issue will be fixed, try the "Test" button because it doesn't seem to work in previous versions

Fixed in the latest master build (v5.1.1)

frantz45 commented 9 months ago

About the old "Alerts" column present in the database, I am unable to find the culprit in the code base: the only way I was able to reproduce is by a call to the REST API (a PUT to endpoint /api/plugins/com.airbus_cyber_security.graylog.wizard/config). Maybe there are some scripts doing just that behind your back? Also, in the case of an upgrade, there should be a migration of the configuration, this can be done with the REST API. I am adding a small note in the changelog and readme about this.

I'm unable to find a culprit on my side too. My script is up to date. Maybe I ran the script before it was updated. Forget this issue, we'll see if it happens again.

airbus-cyber / graylog-plugin-alert-wizard

Compatibility with Graylog v5 #104