[Security Solution] [Sourcerer] KIP Feature Branch Kickoff, remove config index patterns #106460

stephmilovic · 2021-07-21T19:09:58Z

Summary

This is the first stage of the update to Sourcerer to user only Kibana Index Patterns. This update will enable Kibana runtime fields in Security Solution app

This is the feature branch for this update. I am opening a PR against master. Once approved, I will close the PR without merging and open all new feature PRs against this branch. Once the feature is done, we can reopen this PR against master again and merge with minimal testing.

Removes config index patterns from sourcerer and only use Kibana index patterns in its existing functionality
POST a special security-solution KIP from the config index patterns
security-solution KIP becomes the default selection in our app (formerly config index patterns)

Tester: please check the tasks on the issue to ensure your functionality expectations are correct for this stage of development

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-07-21T19:10:00Z

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

elasticmachine · 2021-07-21T19:10:00Z

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

legrego · 2021-07-27T12:17:04Z

x-pack/plugins/security_solution/server/lib/sourcerer/routes/index.ts

+  try {
+    indexPattern = await indexPatternsService.get(DEFAULT_INDEX_PATTERN_ID);
+  } catch (e) {
+    indexPattern = await indexPatternsService.createAndSave({


It looks like you are trying to create the index pattern if it doesn't already exist? There are a lot of reasons why indexPatternsService.get() may fail -- @elastic/kibana-app-services what is the best way to test for a 404/not found response here?

got it taken care of in my follow up PR, will tag you on that code when its up ;P

legrego · 2021-07-27T12:18:37Z

x-pack/plugins/security_solution/server/lib/sourcerer/routes/index.ts

+): Promise<IndexPattern> => {
+  let indexPattern: IndexPattern;
+  try {
+    indexPattern = await indexPatternsService.get(DEFAULT_INDEX_PATTERN_ID);


What happens if a user has an existing index pattern with this id, but mapped to something you don't expect?

that wouldbe a big coincidence. its our reserved id, vs creating an index pattern via Kibana will result in a random id. you'd have to hit the API manually with this specific ID. maybe 0.000001% of users would do that

The index pattern UI allows index patterns to be created with a custom, user-supplied identifier. I agree the chance of this happening is small, but it's probably greater than 0.000001%. This isn't a blocker IMO, but I wanted to call this out as a possibility

I added this as a task on the issue, will make sure to address before merging in master: https://github.com/elastic/security-team/issues/772

got it taken care of in my follow up PR, will tag you on that code when its up ;P

x-pack/plugins/security_solution/server/plugin.ts

x-pack/plugins/security_solution/server/lib/sourcerer/routes/index.ts

stephmilovic · 2021-08-04T14:30:46Z

x-pack/plugins/security_solution/cypress/integration/data_sources/sourcerer.spec.ts

  });

-  describe('Default scope', () => {
+  describe.skip('Default scope', () => {


these were already skipped if thats confusing

stephmilovic · 2021-08-05T13:54:57Z

@elasticmachine merge upstream

stephmilovic · 2021-08-05T14:23:37Z

i dare this test to fail again

…ourcerer_kip

stephmilovic · 2021-08-09T17:01:11Z

@elasticmachine merge upstream

kibanamachine · 2021-08-09T19:48:01Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 19a1e9a
Storybooks Preview
Documentation Changes
Flaky suites:
- xpack-securitySolutionCypressChrome

Test Failures

Kibana Pipeline / general / adds correctly a filter to the global search bar.SearchBar adds correctly a filter to the global search bar

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has failed 19 times on tracked branches: https://github.com/elastic/kibana/issues/69595

AssertionError: Timed out retrying after 60000ms: Expected to find element: `[data-test-subj="comboBoxOptionsList filterFieldSuggestionList-optionsList"] button[title="host.ip"] mark`, but never found it.
    at Object.fillAddFilterForm (http://localhost:61121/__cypress/tests?p=cypress/integration/header/search_bar.spec.ts:16419:8)
    at Context.eval (http://localhost:61121/__cypress/tests?p=cypress/integration/header/search_bar.spec.ts:15554:22)

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`	6.5MB	6.5MB	+1.7KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	211.9KB	212.2KB	+304.0B

History

💔 Build #143687 failed e986d9d
💛 Build #143389 was flaky 32025be
💚 Build #143242 succeeded 11c741d
💔 Build #142967 failed ffc08eb
💔 Build #141137 failed 49098d4

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

michaelolo24 · 2021-08-10T02:09:18Z

x-pack/plugins/security_solution/common/constants.ts

@@ -22,6 +22,8 @@ export const DEFAULT_DATE_FORMAT_TZ = 'dateFormat:tz';
 export const DEFAULT_DARK_MODE = 'theme:darkMode';
 export const DEFAULT_INDEX_KEY = 'securitySolution:defaultIndex';
 export const DEFAULT_NUMBER_FORMAT = 'format:number:defaultPattern';
+export const DEFAULT_INDEX_PATTERN_ID = 'security-solution';


Just curious, is there some standard pattern for these across Kibana? I.e. camel-case, snake-case, etc.. ?

no, the id is generated and looks like a855f700-f5f9-11eb-8763-01917c58cbf9 so i followed that. i think - is standard for URL which is where these are stored

michaelolo24 · 2021-08-10T02:13:28Z

x-pack/plugins/security_solution/cypress/integration/cases/privileges.spec.ts

-      .should('oneOf', [204, 404]);
-  }
+// needed to generate index pattern
+const visitSecuritySolution = () => {


Worth moving into a util? If you end up moving it to a util, should probably rename it to something like loadSecuritySolutionIndexPatterns

michaelolo24 · 2021-08-10T02:20:15Z

x-pack/plugins/security_solution/cypress/integration/data_sources/sourcerer.spec.ts

 // Skipped at the moment as this has flake due to click handler issues. This has been raised with team members
 // and the code is being re-worked and then these tests will be unskipped
-describe.skip('Sourcerer', () => {
-  before(() => {
+describe('Sourcerer', () => {


Thanks for cleaning this up!

michaelolo24 · 2021-08-10T02:49:36Z

x-pack/plugins/security_solution/public/common/components/sourcerer/index.test.tsx

+        kibanaIndexPatterns: [
+          state.sourcerer.defaultIndexPattern,
+          { id: '1234', title: 'auditbeat-*' },
+          { id: '1234', title: 'packetbeat-*' },


Does this get ignored in the selectedPatterns list because auditbeat-* has the same id? Just seeing if that's something you're testing here too

hmm that should never happen IRL since you cant have the same id in the system, so its not something i wrote for

x-pack/plugins/security_solution/public/common/containers/sourcerer/index.tsx

michaelolo24 · 2021-08-10T03:39:46Z

x-pack/plugins/security_solution/public/common/containers/sourcerer/index.tsx

+    const { id, ...rest } = defaultIndexPattern;
+    if (id === null) {
+      // if id is null, rest is error
+      addError(rest, {


I think it might be worth it to set an explicit error param rather than relying on rest here to keep separation of concerns?

im going to iterate on this in follow up PRs but will keep this comment open for a reminder

michaelolo24 · 2021-08-10T03:44:29Z

x-pack/plugins/security_solution/public/common/store/sourcerer/helpers.test.ts

@@ -27,18 +29,18 @@ const ids: Array<Args['id']> = [
 ];
 describe('createDefaultIndexPatterns', () => {
  ids.forEach((id) => {
-    eventTypes.forEach((et) => {
-      describe(`id: ${id}, eventType: ${et}`, () => {
+    eventTypes.forEach((eventType) => {


x-pack/plugins/security_solution/public/common/store/sourcerer/helpers.ts

michaelolo24 · 2021-08-10T03:53:52Z

...gins/security_solution/public/timelines/components/timeline/search_or_filter/pick_events.tsx

-      ].reduce<Array<EuiComboBoxOptionOption<string>>>((acc, index) => {
+      [...kibanaIndexPatterns.map((kip) => kip.title), signalIndexName].reduce<
+        Array<EuiComboBoxOptionOption<string>>
+      >((acc, index) => {
        if (index != null && !acc.some((o) => o.label.includes(index))) {


I know you didn't write this, but isn't it always label === index?

its saying acc shouldn't already have a label that is equal to that index. makes sense to me but we can run it together if you'd like and poke at it 😄

michaelolo24

PR looks good! Pulled it down, tested that the index switching is working as expected. Most of my comments were just questions and nits. Only thing left is just making any changes for @legrego's comment here: https://github.com/elastic/kibana/pull/106460/files#r677392130

stephmilovic · 2021-08-10T14:16:31Z

Fantastic! With approval, I'm now going to close this PR and use it as my Feature Branch to open PRs against. I will reopen the PR once it is ready to merge!

stephmilovic added 7 commits July 19, 2021 11:27

initialize default KIP

4deabab

rm configIndexPatterns

8c452c2

no more config patterns

fde9056

fix sourcerer jest

919a310

another test fix

3b6cf6a

Merge branch 'master' into sourcerer_kip

957bb97

more test fixes

8458b6f

stephmilovic added release_note:enhancement v8.0.0 Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.15.0 labels Jul 21, 2021

stephmilovic requested a review from a team as a code owner July 21, 2021 19:09

stephmilovic added 10 commits July 21, 2021 13:19

rm some comments

c3df135

Merge branch 'master' into sourcerer_kip

0342da6

Merge branch 'master' into sourcerer_kip

af76d50

why

a386a11

fix cy

d10ec88

Merge branch 'master' into sourcerer_kip

33d44f1

merge

e79a9b1

k

3c13e86

move to server

6cc917c

rm cy com

343354d

legrego reviewed Jul 27, 2021

View reviewed changes

stephmilovic added 4 commits July 27, 2021 10:51

ui for permissions and fix tests

86bc00d

pass as arg

255bc1f

fix mistakes

434f9bb

fix types

49098d4

stephmilovic added 3 commits August 3, 2021 12:34

merge master

de97889

fix ml test

ffc08eb

Merge branch 'master' into sourcerer_kip

23fba2c

stephmilovic commented Aug 4, 2021

View reviewed changes

Merge branch 'master' into sourcerer_kip

11c741d

Merge branch 'sourcerer_kip' of github.com:stephmilovic/kibana into s…

32025be

…ourcerer_kip

stephmilovic requested a review from a team August 9, 2021 13:21

fix conflicts

e986d9d

Merge branch 'master' into sourcerer_kip

19a1e9a

michaelolo24 reviewed Aug 10, 2021

View reviewed changes

x-pack/plugins/security_solution/public/common/containers/sourcerer/index.tsx Show resolved Hide resolved

michaelolo24 reviewed Aug 10, 2021

View reviewed changes

x-pack/plugins/security_solution/public/common/store/sourcerer/helpers.ts Show resolved Hide resolved

michaelolo24 reviewed Aug 10, 2021

View reviewed changes

michaelolo24 approved these changes Aug 10, 2021

View reviewed changes

stephmilovic closed this Aug 10, 2021

This was referenced Sep 7, 2021

[Security Solution] [Sourcerer] Update to use Kibana Data Views #111461

Closed

[Security Solution] [Sourcerer] Update to use Kibana Data Views stephmilovic/kibana#4

Closed

stephmilovic mentioned this pull request Oct 13, 2021

[Security Solution] [Sourcerer] [Feature Branch] Update to use Kibana Data Views #114806

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] [Sourcerer] KIP Feature Branch Kickoff, remove config index patterns #106460

[Security Solution] [Sourcerer] KIP Feature Branch Kickoff, remove config index patterns #106460

stephmilovic commented Jul 21, 2021 •

edited

Loading

elasticmachine commented Jul 21, 2021

elasticmachine commented Jul 21, 2021

legrego Jul 27, 2021

stephmilovic Aug 10, 2021

legrego Jul 27, 2021

stephmilovic Jul 27, 2021

legrego Jul 27, 2021

stephmilovic Aug 5, 2021

stephmilovic Aug 10, 2021

stephmilovic Aug 4, 2021

stephmilovic commented Aug 5, 2021

stephmilovic commented Aug 5, 2021

stephmilovic commented Aug 9, 2021

kibanamachine commented Aug 9, 2021

Stack Trace

michaelolo24 Aug 10, 2021

stephmilovic Aug 10, 2021

michaelolo24 Aug 10, 2021

michaelolo24 Aug 10, 2021

michaelolo24 Aug 10, 2021 •

edited

Loading

stephmilovic Aug 10, 2021

michaelolo24 Aug 10, 2021

stephmilovic Aug 10, 2021

michaelolo24 Aug 10, 2021

michaelolo24 Aug 10, 2021

stephmilovic Aug 10, 2021

michaelolo24 left a comment

stephmilovic commented Aug 10, 2021

[Security Solution] [Sourcerer] KIP Feature Branch Kickoff, remove config index patterns #106460

[Security Solution] [Sourcerer] KIP Feature Branch Kickoff, remove config index patterns #106460

Conversation

stephmilovic commented Jul 21, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Jul 21, 2021

elasticmachine commented Jul 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephmilovic commented Aug 5, 2021

stephmilovic commented Aug 5, 2021

stephmilovic commented Aug 9, 2021

kibanamachine commented Aug 9, 2021

💛 Build succeeded, but was flaky

Stack Trace

Metrics [docs]

Async chunks

Page load bundle

History

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 Aug 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

stephmilovic commented Aug 10, 2021

stephmilovic commented Jul 21, 2021 •

edited

Loading

michaelolo24 Aug 10, 2021 •

edited

Loading