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

[Actions] System actions enhancements #161340

Merged
merged 65 commits into from
Jul 28, 2023
Merged

Conversation

cnasikas
Copy link
Member

@cnasikas cnasikas commented Jul 6, 2023

Summary

This PR:

  • Handles the references for system actions in the rule
  • Forbids the creation of system actions through the kibana.yml
  • Adds telemetry for system actions
  • Allow system action types to be disabled from the kibana.yml

Depends on: #160983, #161341

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas cnasikas changed the title [Actions] System actions references [Actions] System actions enhancements Jul 11, 2023
@@ -119,14 +125,17 @@ export async function getInUseTotalCount(
countEmailByService: Record<string, number>;
countNamespaces: number;
}> {
const scriptedMetric = {
const getInMemoryActionScriptedMetric = (actionRefPrefix: string) => ({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switch the order of scriptedMetric and preconfiguredActionsScriptedMetric (which I made it a function)

const inMemoryConnectors = getInMemoryConnectors().filter(
(inMemoryConnector) => inMemoryConnector.isPreconfigured
);
const inMemoryConnectors = getInMemoryConnectors();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will take system actions into account in telemetry.

@cnasikas cnasikas marked this pull request as ready for review July 22, 2023 16:18
@cnasikas cnasikas requested a review from a team as a code owner July 22, 2023 16:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@cnasikas
Copy link
Member Author

@elasticmachine merge upstream

@mikecote mikecote self-requested a review July 25, 2023 18:18
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM! Left a few nits and questions.

(!actionTypeEnabled &&
this.inMemoryConnectors.find((inMemoryConnector) => inMemoryConnector.id === actionId) !==
undefined)
(!actionTypeEnabled && inMemoryConnector !== undefined && inMemoryConnector.isPreconfigured)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nit:

Suggested change
(!actionTypeEnabled && inMemoryConnector !== undefined && inMemoryConnector.isPreconfigured)
inMemoryConnector?.isPreconfigured === true

Comment on lines 82 to 85
* Returns true if action type is enabled or it is a preconfigured action type
* It is possible for to disable an action type but use a preconfigured action
* of action type the disabled one. This does not apply to system actions.
* It should be possible to disable a system action type.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Returns true if action type is enabled or it is a preconfigured action type
* It is possible for to disable an action type but use a preconfigured action
* of action type the disabled one. This does not apply to system actions.
* It should be possible to disable a system action type.
* Returns true if action type is enabled or preconfigured.
* An action type can be disabled but used with a preconfigured action.
* This does not apply to system actions as those can be disabled.

(!actionTypeEnabled &&
this.inMemoryConnectors.find((inMemoryConnector) => inMemoryConnector.id === actionId) !==
undefined)
(!actionTypeEnabled && inMemoryConnector !== undefined && inMemoryConnector.isPreconfigured)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, we should see with @shanisagiv1 if we wan the capability of disabling system actions or not. We can start with the context of the case action (and maybe think of future ones too).

If we are unsure, maybe it's best to hold off in case configuring via kibana.yml isn't the right answer? (ex: what about feature controls, serverless, etc)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed offline and we decided to let the system actions to be disabled from the kibana.yml

Comment on lines 623 to 633
const preConfiguredConnectors = this.inMemoryConnectors.filter(
(connector) => connector.isPreconfigured
);

const isSystemActionAsPreconfiguredInConfig = preConfiguredConnectors.some((connector) =>
this.actionTypeRegistry!.isSystemActionType(connector.actionTypeId)
);

if (isSystemActionAsPreconfiguredInConfig) {
throw new Error('Setting system action types in preconfigured connectors are not allowed');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional nit: in case you want to condense the code

Suggested change
const preConfiguredConnectors = this.inMemoryConnectors.filter(
(connector) => connector.isPreconfigured
);
const isSystemActionAsPreconfiguredInConfig = preConfiguredConnectors.some((connector) =>
this.actionTypeRegistry!.isSystemActionType(connector.actionTypeId)
);
if (isSystemActionAsPreconfiguredInConfig) {
throw new Error('Setting system action types in preconfigured connectors are not allowed');
}
const hasSystemActionAsPreconfiguredInConfig = this.inMemoryConnectors
.filter((connector) => connector.isPreconfigured)
.some((connector) => this.actionTypeRegistry!.isSystemActionType(connector.actionTypeId));
if (hasSystemActionAsPreconfiguredInConfig) {
throw new Error('Setting system action types in preconfigured connectors are not allowed');
}

@cnasikas cnasikas enabled auto-merge (squash) July 28, 2023 10:29
@cnasikas
Copy link
Member Author

@elasticmachine merge upstream

@cnasikas cnasikas merged commit f8a1600 into elastic:main Jul 28, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @cnasikas

@cnasikas cnasikas deleted the system_actions_refs branch July 28, 2023 12:53
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 28, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
## Summary

This PR:
- Handles the references for system actions in the rule
- Forbids the creation of system actions through the `kibana.yml`
- Adds telemetry for system actions
- Allow system action types to be disabled from the `kibana.yml`

Depends on: elastic#160983,
elastic#161341

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Actions/Framework Issues related to the Actions Framework Feature:Actions release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.10.0
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

5 participants