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][Exceptions] - Moves remaining exceptions builder logic into lists plugin #95266

Merged
merged 24 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fc1f774
forking ss autocomplete component to list component
yctercero Mar 22, 2021
11e2a2e
fixed up tests, cypress and unit passing locally
yctercero Mar 23, 2021
db512ab
fixing import
yctercero Mar 23, 2021
d9606b3
updating story type issues
yctercero Mar 23, 2021
279e511
moved over the and/or badge
yctercero Mar 23, 2021
f43dfdc
moved over the and/or badge
yctercero Mar 23, 2021
213ef13
moved exception item
yctercero Mar 23, 2021
45597a9
fix jest unit test
yctercero Mar 23, 2021
b12ad8c
added logic buttons to lists plugin
yctercero Mar 24, 2021
34ee433
moved over main builder logic into lists
yctercero Mar 24, 2021
05adb77
cleaned up builder folder in security solution and tests
yctercero Mar 24, 2021
ba5e490
updating tests, lint issues, etc
yctercero Mar 24, 2021
a578547
fix ups and increase bundle size
yctercero Mar 26, 2021
57d86d2
Merge branch 'master' of github.com:elastic/kibana into rac_builder
yctercero Mar 26, 2021
f55f1b8
fix types
yctercero Mar 26, 2021
e91d08b
fix up mocks in tests
yctercero Mar 26, 2021
d87f432
Merge branch 'master' of github.com:elastic/kibana into rac_builder
yctercero Mar 27, 2021
c951b53
Merge branch 'master' into rac_builder
kibanamachine Mar 29, 2021
47ea537
updated security solution limits
yctercero Mar 29, 2021
cb9a0ca
Merge branch 'master' of github.com:elastic/kibana into rac_builder
yctercero Apr 1, 2021
e4e1785
Merge branch 'master' into rac_builder
kibanamachine Apr 5, 2021
255419d
Merge branch 'master' of github.com:elastic/kibana into rac_builder
yctercero Apr 6, 2021
4a621cb
restoring needed qa test
yctercero Apr 6, 2021
dfb5ce6
restoring needed qa test
yctercero Apr 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pageLoadAssetSize:
lens: 96624
licenseManagement: 41817
licensing: 29004
lists: 202261
lists: 228500
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is expected. Looking at the metrics, the security solution was minus about the same amount as was added to the lists plugin. Since the lists plugin previously did not include UI components, it was able to stay pretty small.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 was curious about that here. Would you mind reducing the securitySolutions limit since that was moved out?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can this added 26,239 be removed from the securitySolution plugin defined here?

I understand it's outside the scope of this PR, but I do want to advocate and ask that your team works to reduce these limits in the near future. At the very highest end, we believe these should be under 100k. The files reported here are served whenever Kibana is loaded, affecting the performance of Kibana as a whole. There are details about these metrics here and details on how to reduce the sizes here. Feel free to reach out if you have any questions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at ways to decrease size. So the example given in the link deals with lazy loading components on register. If there isn't a dedicated exceptions parent page that loads these components, where is the best place to ask that they be lazily loaded?

The lists plugin currently does not register with core.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what you mean by the lists plugin doesn't currently register with core. It's doing that here, which is why this bundle is being generated and included on every page refresh. How is this "lists" plugin currently being used?

Copy link
Contributor

Choose a reason for hiding this comment

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

And we can take this part of the discussion to a dedicated issue. Bringing up the overall bundle sizes is to bring awareness to this increasing problem, which we across Kibana have been trying to reduce for a while now.

As far as this PR, if we have in-fact move logic from the securitySolution to the lists plugin, I would like to see that reflected in the limits file. If we are increasing lists, we should in theory be able to remove that added size from securitySolution.

logstash: 53548
management: 46112
maps: 80000
Expand All @@ -69,7 +69,7 @@ pageLoadAssetSize:
searchprofiler: 67080
security: 189428
securityOss: 30806
securitySolution: 283440
securitySolution: 235402
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tylersmalley Hey! Thanks for chatting with me earlier. I've updated the securitySolution limits here and am investigating how to decrease the lists plugin size.

share: 99061
snapshotRestore: 79032
spaces: 387915
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,4 @@ describe(`enumeratePatterns`, () => {
'src/plugins/charts/public/static/color_maps/color_maps.ts kibana-app'
);
});
it(`should resolve x-pack/plugins/security_solution/public/common/components/exceptions/builder/translations.ts to kibana-security`, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too familiar with these tests but could/should this be replaced with an updated path to the lists location instead of removing it all together?

Copy link
Member

Choose a reason for hiding this comment

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

Hey @dplumlee This test was "unlucky" to randomly select path that you changed. I think you just need to update it with one you have now and we are good. But please don't remove it completely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually me. Thanks for taking a look @dmlemeshko . I'll update path and restore test.

const short = 'x-pack/plugins/security_solution';
const actual = enumeratePatterns(REPO_ROOT)(log)(new Map([[short, ['kibana-security']]]));

expect(
actual[0].includes(
`${short}/public/common/components/exceptions/builder/translations.ts kibana-security`
)
).toBe(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,358 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { Story, addDecorator } from '@storybook/react';
import React from 'react';
import { ThemeProvider } from 'styled-components';
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
import { HttpStart } from 'kibana/public';

import { AutocompleteStart } from '../../../../../../../src/plugins/data/public';
import {
fields,
getField,
} from '../../../../../../../src/plugins/data/common/index_patterns/fields/fields.mocks';
import { getMockTheme } from '../../../common/test_utils/kibana_react.mock';
import { getEntryMatchAnyMock } from '../../../../common/schemas/types/entry_match_any.mock';
import { getEntryMatchMock } from '../../../../common/schemas/types/entry_match.mock';
import { getEntryExistsMock } from '../../../../common/schemas/types/entry_exists.mock';
import { getEntryNestedMock } from '../../../../common/schemas/types/entry_nested.mock';
import { getExceptionListItemSchemaMock } from '../../../../common/schemas/response/exception_list_item_schema.mock';

import {
ExceptionBuilderComponent,
ExceptionBuilderProps,
OnChangeProps,
} from './exception_items_renderer';

const mockTheme = getMockTheme({
darkMode: false,
eui: euiLightVars,
});
const mockHttpService: HttpStart = ({
addLoadingCountSource: (): void => {},
anonymousPaths: {
isAnonymous: (): void => {},
register: (): void => {},
},
basePath: {},
delete: (): void => {},
externalUrl: {
validateUrl: (): void => {},
},
fetch: (): void => {},
get: (): void => {},
getLoadingCount$: (): void => {},
head: (): void => {},
intercept: (): void => {},
options: (): void => {},
patch: (): void => {},
post: (): void => {},
put: (): void => {},
} as unknown) as HttpStart;
const mockAutocompleteService = ({
getValueSuggestions: () =>
new Promise((resolve) => {
setTimeout(() => {
resolve([
'siem-kibana',
'win2019-endpoint-mr-pedro',
'rock01',
'windows-endpoint',
'siem-windows',
'mothra',
]);
}, 300);
}),
} as unknown) as AutocompleteStart;

addDecorator((storyFn) => <ThemeProvider theme={mockTheme}>{storyFn()}</ThemeProvider>);

export default {
argTypes: {
allowLargeValueLists: {
control: {
type: 'boolean',
},
description: '`boolean` - set to true to allow large value lists.',
table: {
defaultValue: {
summary: true,
},
},
type: {
required: true,
},
},
autocompleteService: {
control: {
type: 'object',
},
description:
'`AutocompleteStart` - Kibana data plugin autocomplete service used for field value autocomplete.',
type: {
required: true,
},
},
exceptionListItems: {
control: {
type: 'array',
},
description:
'`ExceptionsBuilderExceptionItem[]` - Any existing exception items - would be populated when editing an exception item.',
type: {
required: true,
},
},
httpService: {
control: {
type: 'object',
},
description: '`HttpStart` - Kibana service.',
type: {
required: true,
},
},
indexPatterns: {
description:
'`IIndexPattern` - index patterns used to populate field options and value autocomplete.',
type: {
required: true,
},
},
isAndDisabled: {
control: {
type: 'boolean',
},
description:
'`boolean` - set to true to disallow users from using the `AND` button to add entries.',
table: {
defaultValue: {
summary: false,
},
},
type: {
required: true,
},
},
isNestedDisabled: {
control: {
type: 'boolean',
},
description:
'`boolean` - set to true to disallow users from using the `Add nested` button to add nested entries.',
table: {
defaultValue: {
summary: false,
},
},
type: {
required: true,
},
},
isOrDisabled: {
control: {
type: 'boolean',
},
description:
'`boolean` - set to true to disallow users from using the `OR` button to add multiple exception items within the builder.',
table: {
defaultValue: {
summary: false,
},
},
type: {
required: true,
},
},
listId: {
control: {
type: 'string',
},
description: '`string` - the exception list id.',
type: {
required: true,
},
},
listNamespaceType: {
control: {
options: ['agnostic', 'single'],
type: 'select',
},
description: '`NamespaceType` - Determines whether the list is global or space specific.',
type: {
required: true,
},
},
listType: {
control: {
options: ['detection', 'endpoint'],
type: 'select',
},
description:
'`ExceptionListType` - Depending on the list type, certain validations may apply.',
type: {
required: true,
},
},
listTypeSpecificIndexPatternFilter: {
description:
'`(pattern: IIndexPattern, type: ExceptionListType) => IIndexPattern` - callback invoked when index patterns filtered. Optional to be used if you would only like certain fields displayed.',
type: {
required: false,
},
},
onChange: {
description:
'`(arg: OnChangeProps) => void` - callback invoked any time builder update to propagate changes up to parent.',
type: {
required: true,
},
},
ruleName: {
description: '`string` - name of the rule list is associated with.',
type: {
required: true,
},
},
},
component: ExceptionBuilderComponent,
title: 'ExceptionBuilderComponent',
};

const BuilderTemplate: Story<ExceptionBuilderProps> = (args) => (
<ExceptionBuilderComponent {...args} />
);

export const Default = BuilderTemplate.bind({});
Default.args = {
allowLargeValueLists: true,
autocompleteService: mockAutocompleteService,
exceptionListItems: [],
httpService: mockHttpService,
indexPatterns: { fields, id: '1234', title: 'logstash-*' },
isAndDisabled: false,
isNestedDisabled: false,
isOrDisabled: false,
listId: '1234',
listNamespaceType: 'single',
listType: 'detection',
onChange: (): OnChangeProps => ({
errorExists: false,
exceptionItems: [],
exceptionsToDelete: [],
}),
ruleName: 'My awesome rule',
};

const sampleExceptionItem = {
...getExceptionListItemSchemaMock(),
entries: [
{ ...getEntryMatchAnyMock(), field: getField('ip').name, value: ['some ip'] },
{ ...getEntryMatchMock(), field: getField('ssl').name, value: 'false' },
{ ...getEntryExistsMock(), field: getField('@timestamp').name },
],
};

const sampleNestedExceptionItem = {
...getExceptionListItemSchemaMock(),
entries: [
{ ...getEntryMatchAnyMock(), field: getField('ip').name, value: ['some ip'] },
{
...getEntryNestedMock(),
entries: [
{
...getEntryMatchMock(),
field: 'child',
value: 'some nested value',
},
],
field: 'nestedField',
},
],
};

const BuilderSingleExceptionItem: Story<ExceptionBuilderProps> = (args) => (
<ExceptionBuilderComponent {...args} />
);

export const SingleExceptionItem = BuilderSingleExceptionItem.bind({});
SingleExceptionItem.args = {
allowLargeValueLists: true,
autocompleteService: mockAutocompleteService,
exceptionListItems: [sampleExceptionItem],
httpService: mockHttpService,
indexPatterns: { fields, id: '1234', title: 'logstash-*' },
isAndDisabled: false,
isNestedDisabled: false,
isOrDisabled: false,
listId: '1234',
listNamespaceType: 'single',
listType: 'detection',
onChange: (): OnChangeProps => ({
errorExists: false,
exceptionItems: [sampleExceptionItem],
exceptionsToDelete: [],
}),
ruleName: 'My awesome rule',
};

const BuilderMultiExceptionItems: Story<ExceptionBuilderProps> = (args) => (
<ExceptionBuilderComponent {...args} />
);

export const MultiExceptionItems = BuilderMultiExceptionItems.bind({});
MultiExceptionItems.args = {
allowLargeValueLists: true,
autocompleteService: mockAutocompleteService,
exceptionListItems: [sampleExceptionItem, sampleExceptionItem],
httpService: mockHttpService,
indexPatterns: { fields, id: '1234', title: 'logstash-*' },
isAndDisabled: false,
isNestedDisabled: false,
isOrDisabled: false,
listId: '1234',
listNamespaceType: 'single',
listType: 'detection',
onChange: (): OnChangeProps => ({
errorExists: false,
exceptionItems: [sampleExceptionItem, sampleExceptionItem],
exceptionsToDelete: [],
}),
ruleName: 'My awesome rule',
};

const BuilderWithNested: Story<ExceptionBuilderProps> = (args) => (
<ExceptionBuilderComponent {...args} />
);

export const WithNestedExceptionItem = BuilderWithNested.bind({});
WithNestedExceptionItem.args = {
allowLargeValueLists: true,
autocompleteService: mockAutocompleteService,
exceptionListItems: [sampleNestedExceptionItem, sampleExceptionItem],
httpService: mockHttpService,
indexPatterns: { fields, id: '1234', title: 'logstash-*' },
isAndDisabled: false,
isNestedDisabled: false,
isOrDisabled: false,
listId: '1234',
listNamespaceType: 'single',
listType: 'detection',
onChange: (): OnChangeProps => ({
errorExists: false,
exceptionItems: [sampleNestedExceptionItem, sampleExceptionItem],
exceptionsToDelete: [],
}),
ruleName: 'My awesome rule',
};
Loading