-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Cases][Alerting] Validate actions using additional fields beyond params #116006
Comments
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
Pinging @elastic/security-threat-hunting-cases (Team:Threat Hunting:Cases) |
@arisonl Do you feel this is something we want to enforce at platform level? |
@jonathan-buttner would it be possible to share more on the product thinking around this? E.g. this may force users to operate with a mix of rules using the deprecated and new connector until they "migrate" all rules to the new connector, right? Is there any chance that this may cause workflow/operational friction? Thoughts on the alternative of not prohibiting but rather strongly discouraging through the UX? cc @paulewing @cnasikas @vinaychandrasekhar @gmmorris |
Yeah @paulewing @cnasikas @alexfrancoeur could probably speak to that better. With the certification of service now I think we're trying to encourage users to update their connectors and use the elastic service now application. I can definitely see your point about how it could be frustrating to lose some functionality suddenly when they upgrade though.
I think the hope is that they'd update all their connectors (which is hopefully not that many) rather than all their rules. A user who had many rules using one or two different service now connectors would only need to update their connectors to the latest version (which includes installing the elastic service now application on their service now instance).
@paulewing @cnasikas should we change cases to allow creating cases with the deprecated connectors and allow changing the deprecated connectors that are already associated with a case? |
As @jonathan-buttner said, I think the reason behind this decision for cases is to enforce users to migrate to the new connector.
Maybe we should revise. Let's talk about it on the sync. |
Pinging @elastic/response-ops-cases (Feature:Cases) |
Background
We've introduced the concept of a "legacy/deprecated" connector in #105440. Currently we've added a deprecated icon to the connectors list page
Deprecated Connectors List Page
We'd like to show the deprecation message within the rule edit/creation UI like so:
Deprecated Callout in edit/create UI
The Cases plugin currently prevents a user from creating a new case when they've selected a deprecated connector for the case. Cases also does not allow fields within the connector to be updated (like the
priority
field) if it is deprecated. Users can still make changes to the case, add comments, etc, and push the case to service now even if the connector is deprecated.The problem
We'd like to prevent new rules from being created if the selected connector is deprecated. Ideally we wouldn't prevent editing a rule that contains a deprecated connector. Allowing a user to edit a rule will still give them the flexibility to make updates to their rules after upgrading to 7.16 even if they haven't been able to install the elastic service now application.
Currently the field that controls whether a connector is deprecated is stored in the
config
section. As far as I can tell theconfig
section is not used to validate an action during the alert creation/editing flow:https://github.com/elastic/kibana/blob/master/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx#L134
The
alert
parameter contains theparams
but not theconfig
portion from a connector.Potential Solutions
Add state bucket
I believe the component that displays the available connectors uses a callback to set the chosen
id
of the connector and stores it in theAlertAction
fields:https://github.com/elastic/kibana/blob/master/x-pack/plugins/alerting/common/alert.ts#L50
One option would be to add an additional field that can be set by the child component that is also accessible by
AlertAdd
andAlertEdit
so it can be passed to a validation function managed by each connector.I think this solution has a lot of overlap with this issue which talks about adding another bucket of values (state): #114148
Store the configs in the upper
AlertAdd
andAlertEdit
componentsAnother solution would be to add another reducer here: https://github.com/elastic/kibana/blob/master/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx#L68
which the child component could update using
dispatch
like it does for setting theid
of the connector. The child component would dispatch an action to set the config of the selected connectors. This solution might avoid needing an additional bucket of state.Add the deprecation flag to params
This is probably more of a hack. We could add the deprecation flag into the params field as well. We already have access to the config section within the service now params components: https://github.com/elastic/kibana/blob/master/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/servicenow/servicenow_sir_params.tsx#L46
We could then set the deprecation flag in a
useEffect
so it wouldn't be dependent on any user filled field in the params component.We could then add additional validation logic to service nows
validateParams
function so that it would return an error if the deprecation field is ever set totrue
. The downside here is that this won't satisfy allowing users to edit rules if they are already using a deprecated connector (it being marked deprecated after the 7.16 upgrade migration).Another downside to this solution is that I believe all the params are set during an
_execute
call. We'd need to edit the service now action plugin's backend implementation to strip off that field before making requests to service now since it's not a valid service now API field.Allowing edits to a rule with a deprecated connector
To allow users to edit existing rules that now have a deprecated connector we'll need additional validation logic.
I think if we an additional
state
bucket, we could add a newvalidateState
function that is optional in theActionTypeModel
https://github.com/elastic/kibana/blob/master/x-pack/plugins/triggers_actions_ui/public/types.ts#L128During the rule creation/edit flow we could use
validateState
if it was defined.validateState
could take an operation parameter so that for service now we could allow rule edits event for a deprecated connector but prevent creating new rules.If we went with adding the deprecation flag to the params, we could also add a new parameter to the
validateParams
so that we could figure out if the operation was an edit or create during validation.Related Issues
Copy update: https://github.com/elastic/security-team/issues/1938
New
state
bucket: #114148Original bug: #114097
The text was updated successfully, but these errors were encountered: