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 #173626

Closed
8 tasks done
Tracked by #174168
approksiu opened this issue Dec 19, 2023 · 15 comments
Closed
8 tasks done
Tracked by #174168

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

approksiu opened this issue Dec 19, 2023 · 15 comments
Assignees
Labels
8.14 candidate 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 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

Comments

@approksiu
Copy link

approksiu commented Dec 19, 2023

Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168
Related to: https://github.com/elastic/security-team/issues/6951, https://github.com/elastic/enhancements/issues/15982

Background

Users currently cannot edit the Setup guide field for their custom rules in UI. This prevents them from easily adding relevant information about their rules - they either need to import rules with this field setup, or use the other available text edit fields for this purpose.

User Story / Problem Statement(s)

  • Expose the Setup guide field in the Rule edit/create page under Advanced settings.

Designs

https://www.figma.com/file/4OzwpyZYgb7haOKQlSZBql/Allow-users-to-add%2Fedit-Setup-guide-field-for-rules-%236951?type=design&node-id=1%3A40&mode=design&t=jwrbC9hVoYCdmd1S-1

Release progress

  • Initial implementation is done. A decision is made regarding using a feature flag vs a long-living feature branch for this feature. (PR)
  • Feature is covered with automated tests. (PR, PR)
  • Feature is fully implemented and considered by the development team as ready to be released.
  • Acceptance testing is done and the feature is approved by @approksiu and @ARWNightingale.
  • Exploratory testing is done and the feature is approved by @vgomez-el.
  • Documentation is written for ESS and Serverless by @joepeeples. Two docs PRs are approved and ready to be merged. (ticket)
  • Feature is released in Serverless.

Planned release date in Serverless: the week of April 15, 2024.
Planned release date in ESS: May 21, 2024 (v8.14.0).

@approksiu
Copy link
Author

The Setup guide component should not have the "interactive actions", only editing capability.

@banderror banderror changed the title [Security Solution] Allow users to edit setup field for rules [Security Solution] Allow users to edit setup field for custom rules Mar 4, 2024
@dplumlee dplumlee self-assigned this Mar 4, 2024
@banderror banderror added enhancement New value added to drive a business result Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Rule Creation Security Solution Detection Rule Creation workflow 8.14 candidate labels Mar 7, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@banderror
Copy link
Contributor

Hey @dplumlee @maximpn @nikitaindik, just a comment regarding all the 4 tickets: #173595, #173594, #173593, and #173626

I triaged and added a Release progress checklist to each of them. There are two concerns I've already discussed with most of you today:

  • Since we need to introduce some schema changes for each of these tickets, it's not 100% clear if we should use the tried and proven approach of feature toggling to release these enhancements, or we'd have to do it using feature branches, or combine the two approaches. Please think of pros and cons of these approaches and let's discuss on Monday how we're going to proceed.
  • We probably don't need to write a new test plan for these enhancements, because there are no existing test plans for rule creation or editing, and because this is not our area of ownership. But I believe we need to add tests anyway. Let's also align on that on Monday and see what kind of tests we'd need to add.

@jpdjere
Copy link
Contributor

jpdjere commented Mar 11, 2024

cc @approksiu @banderror

Discussed release approach with the @elastic/security-detection-rule-management team and @joepeeples for Docs:

  • The schema changes introduced in these four upcoming PRs follow the pattern of: adding each respective field to either the BaseOptionalFields or BaseDefaultableFields and removing them from ResponseFields. This means that, when creating or editing a rule, the API response will stay the same, while the request payload now no longer ignores the new field and uses it to create/update the field.
    • In practice, the only breaking change we can see here is an edge case in which a user is currently using the API to edit a rule and includes in the payload one of the newly customizable fields, trusting that it will be ignored by the endpoint logic. If the feature is released, that will no longer be the case.
  • Creating feature toggles for releasing these features would be a high complexity for the edge case that we're trying to prevent: we would need to split up our rule schema and offer the legacy or modified version depending on the toggle.
  • A long-lived feature branch which accumulated all changes in all four tickets (making all four fields customisable) would be hard to maintain, as it would take a considerable amount of time until all PRs were merged and ready to review, and merge back to main. The branch would need constant merges/rebases from main.
  • Instead, and in agreement with @joepeeples, we will have each PR author coordinate the release of each feature with the Docs team so that the time that the feature change is live with no documentation is minimised or completely eliminated.

@banderror
Copy link
Contributor

we will have each PR author coordinate the release of each feature with the Docs team so that the time that the feature change is live with no documentation is minimised or completely eliminated.

Sounds good to me 👍 Thanks a lot for the update @jpdjere!

@dplumlee
Copy link
Contributor

Deployment links have been added to the PR in the latest build message for acceptance and exploratory testing. cc @vgomez-el @approksiu

@banderror
Copy link
Contributor

Hey all, @dplumlee's PR was approved about 3 weeks ago from the engineering side and since then it has been waiting for other release-related activities. I'd like to check where are we in the release process and if we're able to release this enhancement next week.

@approksiu As far as I understand you acceptance tested this and we got an approval from you?

@ARWNightingale @vgomez-el @joepeeples How's it going on your side?

We're targeting next Monday, April 8, as the date of release to Serverless. On Monday the release pipeline will start and the release to Serverless production will happen in a couple of days after that. This means that we would like to merge this PR by the end of this Friday, April 5, Pacific Time Zone.

@ARWNightingale @vgomez-el Can you please confirm that you're able to complete acceptance and exploratory testing by EOD April 5, or otherwise let me know how much more time you need?

Also @joepeeples can you please confirm that the docs will be ready by Tuesday, April 9?

@vgomez-el
Copy link

Apologies for the late response. I didn't notice about the comments on this ticket.
I have performed the exploratory testing over the feature in the cloud environment created on the PR. Even I will need to perform more tests for ESS when the feature is merged, it is looking good for me so far.

@banderror banderror added Feature:Rule Edit Security Solution Detection Rule Editing workflow v8.14.0 labels Apr 8, 2024
banderror pushed a commit that referenced this issue Apr 8, 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)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5603)

### Screenshots
![Screenshot 2024-03-08 at 11 12
25 AM](https://github.com/elastic/kibana/assets/56367316/5a00b007-d02d-4f1e-b1ba-ca7ba7f68bbd)


![Screenshot 2024-03-06 at 10 25
47 AM](https://github.com/elastic/kibana/assets/56367316/a3973e10-1c82-4981-b38d-69faf06a5993)


### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))


### 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)

---------

Co-authored-by: Kibana Machine <[email protected]>
@banderror
Copy link
Contributor

@dplumlee @joepeeples /cc @vgomez-el @approksiu @ARWNightingale

The PR that we merged on Monday was merged too late and didn't make it to the release this week. The 7abe4c2b5ac8 commit went live which is an older one:

Screenshot_2024-04-09_at_20_38_28

So the enhancement should go live next week, Monday Apr 15th.

@joepeeples
Copy link
Contributor

joepeeples commented Apr 12, 2024

Also @joepeeples can you please confirm that the docs will be ready by Tuesday, April 9?

@banderror Putting this at the top of my list for next week (week of April 15) to try to get serverless docs published on, or around, April 16. TBH so far I haven't been able to work on this yet; it's been hard to adjust to documenting features so far out of the usual release cycle, but we'll get there. Also sorry I missed this earlier; I've been pinged on a lot of active PRs and issues recently and it's been very hard to keep up.

@banderror
Copy link
Contributor

@joepeeples Got it, sorry I didn't know about the pressure we were contributing to. Then, I would suggest to re-target our other fields UI work (related integrations, required fields, maybe even max signals) to 8.15.0 to make more room for writing documentation and for testing. I'll raise this topic at the team sync.

@approksiu
Copy link
Author

@banderror Setup guide has the highest priority. I am ok with the other fields being released next time.

@joepeeples
Copy link
Contributor

joepeeples commented Apr 15, 2024

@banderror and @approksiu Thanks for being flexible. It turns out the docs we needed for the UI were pretty minimal, so I was able to open the serverless and classic/ESS PRs today. We should be on track to publish tomorrow (Apr 16) to coincide with the field's release to serverless prod.

API docs update might take a little longer. We can discuss in the next team sync.

@banderror
Copy link
Contributor

The feature went live in Serverless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.14 candidate 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 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

No branches or pull requests

7 participants