[SIEM] New Platform Cutover #59624
Closed
[SIEM] New Platform Cutover #59624
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Throwing a TypeError meant that our manual errors were indistinguishable from, say, trying to invoke a method on undefined. This adds a custom error, BadRequestError, that disambiguates that situation.
With Core's new HTTP client, an unsuccessful API call will raise an error containing the body of the response it received. In the case of SIEM endpoints, this will include a useful error message with potentially more specificity than e.g. 'Internal Server Error'. This adds a type predicate to check for such errors, and adds a handling case in our errorToToaster handler. If the error does not contain our SIEM-specific message, it will fall through as normal and the general error.message will be displayed in the toaster.
The new HTTP client raises an error on a 4xx or 5xx response, so there should not be a case where throwIfNotOk is actually going to throw an error. The established pattern on the frontend is to catch errors at the call site and handle them appropriately, so I'm mainly just verifying that these are caught where they're used, now.
These were living in ML since that's where they originated. However, we have need of it (and already use it) elsewhere. The basic pattern for error handling on the frontend is: 1) API call throws error 2) caller catches error and dispatches a toast throwIfNotOk is meant to convert the error into a useful message in 1). We currently use both errorToToaster and displayErrorToast to display that in a toaster in 2) Now that errorToToaster handles a few different types of errors, and throwIfNotOk is going to be bypassed due to the new client behavior of throwing on error, we're going to start consolidating on: 1) Api call throws error 2) caller catches error and passes it to errorToToaster
* Ensures that all callers of these methods properly catch errors * Updates error toasterification to use errorToToaster * Simplifies tests now that we mainly just invoke the http client and return the result. throwIfNotOk is not being used in the majority of cases, as the client raises an error and bypasses that call. The few cases this might break are where we return a 200 but have errors within the response. Whether throwIfNotOk handled this or not, I'll need a simpler helper to accomplish the same behavior.
These can be an array of errors OR rules; typing it as such forces downstream to deal with both. enableRules was being handled correctly with the bucketing helper, and TS has confirmed the rest are as well. This obviates the need to raise from our API calls, as bulk errors are recoverable and we want to both a) continue on with any successful rules and b) handle the errors as necessary. This is highly dependent on the caller and so we can't/shouldn't handle it here.
I'm not sure that we're ever using this non-dispatch version, but it was throwing a type error. Will bring it up in review.
These are unneeded as an error response will already throw an error to be handled at the call site.
This was left over as a result of elastic#56261
Again, not needed because the client already throws.
* Gets rid of throwIfNotOK usage * uses core http fetch
This served the same purpose as errorToToaster, but in a less robust way. All usages have been replaced, so now we say goodbye.
…redicate There was no functional difference between these two code paths, and removing these custom errors allowed us to delete a bunch of associated code as well..
These were mainly related to my swapping any remaining fetch calls with the core router as good kibana denizens should :salute:
This is enough to get our tests to pass. We can't use the core mocks for now since there are circular dependencies there, which breaks our build.
* Simplifies our mocking verbosity by leveraging core mocks * Simplifies test setup by isolating a reference to our fetch mock * Abstracts response structure to pure helper functions The try/catch tests had some false positives in that nothing would be asserted if the code did not throw an error. These proved to be masking a gap in coverage for our get/create signal index requests, which do not leverage `throwIfNotOk` but instead rely on the fetch to throw an error; once that behavior is verified we can update those tests to have our fetchMock throw errors, and we should be all set.
We no longer re-throw errors, or parse the response, we just return the result of the client call. Simple!
* Updates plugin to use NP config * defines new config previously coming from savedObjects config * Declares dependencies in kibana.json * all required, for now * cleans up legacy types
* saved object types are not currently registered * all our ui/chrome imports are failing * other trickier stuff, but we've got an error list to work through.
I don't know if we were trying to discourage its use, but it's another uiSetting default so it's going with its siblings.
* One is a fetch that somehow made it through, and the other is the retrieval of the kibana version. Both were replaced with KibanaServices.
This has been replaced with NP service calls.
That module includes references to legacy code that breaks our build.
This is a named export, whoops
* Replace build-breaking ui/new_platform mocks with equivalents in core proper * Remove unnecessary mocks of ui/new_platform
This breaks our build. I'm not sure what the analog here is.
... Conflicts: x-pack/legacy/plugins/siem/public/components/ml/api/error_to_toaster.ts x-pack/legacy/plugins/siem/public/components/ml_popover/api.tsx x-pack/legacy/plugins/siem/public/components/toasters/utils.test.ts x-pack/legacy/plugins/siem/public/containers/case/api.ts x-pack/legacy/plugins/siem/public/containers/case/use_get_tags.tsx x-pack/legacy/plugins/siem/public/containers/case/use_post_case.tsx x-pack/legacy/plugins/siem/public/containers/case/use_post_comment.tsx x-pack/legacy/plugins/siem/public/containers/case/use_update_case.tsx x-pack/legacy/plugins/siem/public/containers/case/use_update_comment.tsx x-pack/legacy/plugins/siem/public/containers/detection_engine/rules/api.test.ts x-pack/legacy/plugins/siem/public/containers/detection_engine/signals/errors_types/get_index_error.ts x-pack/legacy/plugins/siem/public/containers/detection_engine/signals/errors_types/post_index_error.ts x-pack/legacy/plugins/siem/public/containers/detection_engine/signals/errors_types/privilege_user_error.ts x-pack/legacy/plugins/siem/public/hooks/api/api.tsx x-pack/legacy/plugins/siem/public/hooks/api/throw_if_not_ok.test.ts x-pack/legacy/plugins/siem/public/hooks/api/throw_if_not_ok.ts
Conflicts: x-pack/legacy/plugins/siem/public/components/ml/api/error_to_toaster.test.ts x-pack/legacy/plugins/siem/public/components/toasters/utils.test.ts x-pack/legacy/plugins/siem/public/containers/detection_engine/signals/errors_types/index.ts x-pack/legacy/plugins/siem/server/saved_objects.ts x-pack/plugins/siem/public/components/ml/api/error_to_toaster.test.ts x-pack/plugins/siem/public/components/ml/api/error_to_toaster.ts x-pack/plugins/siem/public/components/news_feed/index.tsx x-pack/plugins/siem/public/components/recent_timelines/helpers.ts x-pack/plugins/siem/public/containers/case/use_get_tags.tsx x-pack/plugins/siem/public/containers/detection_engine/signals/errors_types/get_index_error.ts x-pack/plugins/siem/public/containers/detection_engine/signals/errors_types/index.ts x-pack/plugins/siem/public/containers/detection_engine/signals/errors_types/post_index_error.ts x-pack/plugins/siem/public/containers/detection_engine/signals/errors_types/privilege_user_error.ts x-pack/plugins/siem/public/hooks/api/api.test.ts x-pack/plugins/siem/server/lib/detection_engine/routes/utils.ts x-pack/plugins/transform/public/index.ts
rylnd
added
Feature:New Platform
Team:SIEM
v8.0.0
release_note:skip
|
Pinging @elastic/siem (Team:SIEM) |
Conflicts: x-pack/legacy/plugins/siem/public/components/ml/api/error_to_toaster.test.ts x-pack/legacy/plugins/siem/public/components/toasters/utils.test.ts x-pack/legacy/plugins/siem/public/hooks/api/throw_if_not_ok.test.ts x-pack/legacy/plugins/siem/public/hooks/api/throw_if_not_ok.ts x-pack/legacy/plugins/siem/public/utils/api/index.ts x-pack/legacy/plugins/siem/server/lib/detection_engine/errors/bad_request_error.ts x-pack/plugins/siem/public/components/ml/api/throw_if_not_ok.test.ts x-pack/plugins/siem/public/components/ml/api/throw_if_not_ok.ts x-pack/plugins/siem/public/components/ml_popover/api.tsx x-pack/plugins/siem/public/components/toasters/errors.ts x-pack/plugins/siem/public/components/toasters/throw_if_not_ok.test.ts x-pack/plugins/siem/public/components/toasters/throw_if_not_ok.ts x-pack/plugins/siem/public/components/toasters/utils.test.ts x-pack/plugins/siem/public/containers/case/use_post_case.tsx x-pack/plugins/siem/public/containers/case/use_post_comment.tsx x-pack/plugins/siem/public/containers/case/use_update_case.tsx x-pack/plugins/siem/public/containers/case/utils.ts x-pack/plugins/siem/public/containers/detection_engine/rules/api.ts x-pack/plugins/siem/public/containers/detection_engine/signals/api.ts x-pack/plugins/siem/server/lib/detection_engine/routes/utils.ts
This import does not break the build; my mistake
💔 Build FailedTest FailuresKibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/dashboard/dashboard_query_bar·js.dashboard app using current data dashboard query bar causes panels to reload when refresh is clickedStandard OutStack TraceHistory
To update your PR or re-run it, just comment with: |
26 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This moves both the frontend and the backend over to NP. While maps are broken on this branch (#58178), moving the entirety of SIEM at once was significantly easier since it didn't require modification of any internal, relative imports. If we decide that the frontend can't be moved in 7.7 I do have a WIP branch for that, but my preference is to move them both if possible.
There were two main tasks involved in this branch:
NB Regarding review: discounting renames, this branch is ~+1000/-1000 LOC. GitHub (and git) do not by default track file renames across this many files. I highly recommend checking this branch out locally and temporarily setting e.g.
git config diff.renameLimit 2500to see a meaningful diff.Status
With the exception of maps, this branch is "working" in that it runs without error and I've not yet found any unexpected behavior.
Outstanding tasks/issues
any, which is causing both TS and test errors.request.headers.isAuthenticatedfunctionality ([SIEM] Enrich Privileges endpoint with NP Authentication data #59225)Checklist
Delete any items that are not applicable to this PR.
For maintainers