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

[Security Solution] Allow users to edit setup field for custom rules #178131

Merged
merged 15 commits into from
Apr 8, 2024

Conversation

dplumlee
Copy link
Contributor

@dplumlee dplumlee commented Mar 6, 2024

Summary

Addresses #173626

Adds a markdown component in the create and edit rule forms so that users are able to add their own setup guides to custom rules. Also updates the create and update rule schemas and route logic to handle these new cases through the API.

Flaky test run (internal)

Screenshots

Screenshot 2024-03-08 at 11 12 25 AM

Screenshot 2024-03-06 at 10 25 47 AM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@dplumlee dplumlee added release_note:enhancement Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Rule Management Security Solution Detection Rule Management area Team:Detection Rule Management Security Detection Rule Management Team v8.14.0 labels Mar 6, 2024
@dplumlee dplumlee self-assigned this Mar 6, 2024
@dplumlee
Copy link
Contributor Author

dplumlee commented Mar 6, 2024

/ci

@dplumlee dplumlee marked this pull request as ready for review March 6, 2024 22:28
@dplumlee dplumlee requested review from a team as code owners March 6, 2024 22:28
@dplumlee dplumlee requested review from rylnd and jpdjere March 6, 2024 22:28
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@jpdjere
Copy link
Contributor

jpdjere commented Mar 7, 2024

In x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts, we have the following check:

export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise<InternalRuleCreate> => {
  // Generate a new static ruleId
  const ruleId = uuidv4();

  // If it's a prebuilt rule, reset Related Integrations, Required Fields and Setup Guide.
  // We do this because for now we don't allow the users to edit these fields for custom rules.
  const isPrebuilt = rule.params.immutable;
  const relatedIntegrations = isPrebuilt ? [] : rule.params.relatedIntegrations;
  const requiredFields = isPrebuilt ? [] : rule.params.requiredFields;
  const setup = isPrebuilt ? '' : rule.params.setup;
[...]

We can get rid of that check for setup now.

@jpdjere
Copy link
Contributor

jpdjere commented Mar 7, 2024

In https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts

The types of the arguments of convertCreateAPIToInternalSchema and convertPatchAPIToInternalSchema can be simplified:

export const convertCreateAPIToInternalSchema = (
  input: RuleCreateProps & {
    related_integrations?: RelatedIntegrationArray;
    required_fields?: RequiredFieldArray;
    setup?: SetupGuide; /// <------- this can be removed since setup is now part of RuleCreateProps
  },
  immutable = false,
  defaultEnabled = true
): InternalRuleCreate => {
export const convertPatchAPIToInternalSchema = (
  nextParams: PatchRuleRequestBody & {
    related_integrations?: RelatedIntegrationArray;
    required_fields?: RequiredFieldArray;
    setup?: SetupGuide; /// <------- this can be removed since setup is now part of PatchRuleRequestBody
  },
  existingRule: SanitizedRule<RuleParams>
): InternalRuleUpdate => {

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

@jpdjere asked the hard questions, here, but beyond those this LGTM!

@dplumlee dplumlee requested a review from jpdjere March 8, 2024 16:13
@approksiu
Copy link

Looks good! Thanks @dplumlee!

@banderror
Copy link
Contributor

@elasticmachine merge upstream

@dplumlee
Copy link
Contributor Author

dplumlee commented Apr 8, 2024

@elasticmachine merge upstream

@banderror
Copy link
Contributor

@elasticmachine merge upstream

@banderror banderror enabled auto-merge (squash) April 8, 2024 10:37
@banderror banderror changed the title [Security Solution] Setup field form component [Security Solution] Allow users to edit setup field for custom rules Apr 8, 2024
@banderror banderror added Feature:Rule Creation Security Solution Detection Rule Creation workflow Feature:Rule Edit Security Solution Detection Rule Editing workflow and removed Feature:Rule Management Security Solution Detection Rule Management area labels Apr 8, 2024
@kibana-ci
Copy link
Collaborator

kibana-ci commented Apr 8, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 17.0MB 17.0MB +4.7KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5847 +5847
total size - 6.7MB +6.7MB

History

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

cc @dplumlee

@banderror banderror merged commit 9f8433e into elastic:main Apr 8, 2024
40 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 8, 2024
@dplumlee dplumlee deleted the setup-field-form-component branch April 8, 2024 14:16
@jpdjere jpdjere self-requested a review April 10, 2024 11:54
@jpdjere
Copy link
Contributor

jpdjere commented Apr 12, 2024

Crossposting #179680 (comment). Follow up PR needed for a schema update.

cc @dplumlee

@banderror banderror added release_note:feature Makes this part of the condensed release notes enhancement New value added to drive a business result and removed release_note:enhancement labels Apr 13, 2024
dplumlee added a commit that referenced this pull request May 2, 2024
…p guide field (#180638)

## Summary

Adds extra tests to cover remaining areas not addressed in
#178131

Adds cypress tests and adds `setup` field to utils to be used in
import/export integration tests

[Flaky test run
(internal)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5776)

### 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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed



### For maintainers

- [ ] 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)
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 ci:cloud-deploy Create or update a Cloud deployment ci:project-deploy-security Create a Security Serverless Project enhancement New value added to drive a business result Feature:Rule Creation Security Solution Detection Rule Creation workflow Feature:Rule Edit Security Solution Detection Rule Editing workflow release_note:feature Makes this part of the condensed release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants