-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Changes from 16 commits
fc1f774
11e2a2e
db512ab
d9606b3
279e511
f43dfdc
213ef13
45597a9
b12ad8c
34ee433
05adb77
ba5e490
a578547
57d86d2
f55f1b8
e91d08b
d87f432
c951b53
47ea537
cb9a0ca
e4e1785
255419d
4a621cb
dfb5ce6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,7 @@ pageLoadAssetSize: | |
lens: 96624 | ||
licenseManagement: 41817 | ||
licensing: 39008 | ||
lists: 202261 | ||
lists: 228500 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
logstash: 53548 | ||
management: 46112 | ||
maps: 183610 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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`, () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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,8 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
export const getEmptyValue = (): string => '—'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/* | ||
* 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 { RecursivePartial } from '@elastic/eui/src/components/common'; | ||
|
||
import { EuiTheme } from '../../../../../../src/plugins/kibana_react/common'; | ||
|
||
export const getMockTheme = (partialTheme: RecursivePartial<EuiTheme>): EuiTheme => | ||
partialTheme as EuiTheme; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
/* | ||
* 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 { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; | ||
|
||
import { getMockTheme } from '../../../common/test_utils/kibana_react.mock'; | ||
|
||
import { AndOrBadge, AndOrBadgeProps } from '.'; | ||
|
||
const sampleText = | ||
'Doggo ipsum i am bekom fat snoot wow such tempt waggy wags floofs, ruff heckin good boys and girls mlem. Ruff heckin good boys and girls mlem stop it fren borkf borking doggo very hand that feed shibe, you are doing me the shock big ol heck smol borking doggo with a long snoot for pats heckin good boys. You are doing me the shock smol borking doggo with a long snoot for pats wow very biscit, length boy. Doggo ipsum i am bekom fat snoot wow such tempt waggy wags floofs, ruff heckin good boys and girls mlem. Ruff heckin good boys and girls mlem stop it fren borkf borking doggo very hand that feed shibe, you are doing me the shock big ol heck smol borking doggo with a long snoot for pats heckin good boys.'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. excellent |
||
|
||
const mockTheme = getMockTheme({ | ||
darkMode: false, | ||
eui: euiLightVars, | ||
}); | ||
|
||
addDecorator((storyFn) => <ThemeProvider theme={mockTheme}>{storyFn()}</ThemeProvider>); | ||
|
||
export default { | ||
argTypes: { | ||
includeAntennas: { | ||
control: { | ||
type: 'boolean', | ||
}, | ||
description: 'Determines whether extending vertical lines appear extended off of round badge', | ||
table: { | ||
defaultValue: { | ||
summary: false, | ||
}, | ||
}, | ||
type: { | ||
required: false, | ||
}, | ||
}, | ||
type: { | ||
control: { | ||
options: ['and', 'or'], | ||
type: 'select', | ||
}, | ||
description: '`and | or` - determines text displayed in badge.', | ||
table: { | ||
defaultValue: { | ||
summary: 'and', | ||
}, | ||
}, | ||
type: { | ||
required: true, | ||
}, | ||
}, | ||
}, | ||
component: AndOrBadge, | ||
title: 'AndOrBadge', | ||
}; | ||
|
||
const AndOrBadgeTemplate: Story<AndOrBadgeProps> = (args) => ( | ||
<EuiFlexGroup> | ||
<EuiFlexItem grow={false}> | ||
<AndOrBadge {...args} /> | ||
</EuiFlexItem> | ||
<EuiFlexItem> | ||
<p>{sampleText}</p> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
); | ||
|
||
export const Default = AndOrBadgeTemplate.bind({}); | ||
Default.args = { | ||
includeAntennas: false, | ||
type: 'and', | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/* | ||
* 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 React from 'react'; | ||
import { ThemeProvider } from 'styled-components'; | ||
import { mount } from 'enzyme'; | ||
|
||
import { getMockTheme } from '../../../common/test_utils/kibana_react.mock'; | ||
|
||
import { AndOrBadge } from './'; | ||
|
||
const mockTheme = getMockTheme({ eui: { euiColorLightShade: '#ece' } }); | ||
|
||
describe('AndOrBadge', () => { | ||
test('it renders top and bottom antenna bars when "includeAntennas" is true', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<AndOrBadge includeAntennas type="and" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('AND'); | ||
expect(wrapper.find('[data-test-subj="andOrBadgeBarTop"]').exists()).toBeTruthy(); | ||
expect(wrapper.find('[data-test-subj="andOrBadgeBarBottom"]').exists()).toBeTruthy(); | ||
}); | ||
|
||
test('it does not render top and bottom antenna bars when "includeAntennas" is false', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<AndOrBadge type="or" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('OR'); | ||
expect(wrapper.find('[data-test-subj="andOrBadgeBarTop"]').exists()).toBeFalsy(); | ||
expect(wrapper.find('[data-test-subj="andOrBadgeBarBottom"]').exists()).toBeFalsy(); | ||
}); | ||
|
||
test('it renders "and" when "type" is "and"', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<AndOrBadge type="and" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('AND'); | ||
}); | ||
|
||
test('it renders "or" when "type" is "or"', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<AndOrBadge type="or" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('OR'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/* | ||
* 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 React from 'react'; | ||
|
||
import { RoundedBadge } from './rounded_badge'; | ||
import { RoundedBadgeAntenna } from './rounded_badge_antenna'; | ||
|
||
export type AndOr = 'and' | 'or'; | ||
export interface AndOrBadgeProps { | ||
type: AndOr; | ||
includeAntennas?: boolean; | ||
} | ||
/** Displays AND / OR in a round badge */ | ||
// This ticket is closed, however, as of 3/23/21 no round badge yet | ||
// Ref: https://github.com/elastic/eui/issues/1655 | ||
export const AndOrBadge = React.memo<AndOrBadgeProps>(({ type, includeAntennas = false }) => { | ||
return includeAntennas ? <RoundedBadgeAntenna type={type} /> : <RoundedBadge type={type} />; | ||
}); | ||
|
||
AndOrBadge.displayName = 'AndOrBadge'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/* | ||
* 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 React from 'react'; | ||
import { mount } from 'enzyme'; | ||
|
||
import { RoundedBadge } from './rounded_badge'; | ||
|
||
describe('RoundedBadge', () => { | ||
test('it renders "and" when "type" is "and"', () => { | ||
const wrapper = mount(<RoundedBadge type="and" />); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('AND'); | ||
}); | ||
|
||
test('it renders "or" when "type" is "or"', () => { | ||
const wrapper = mount(<RoundedBadge type="or" />); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('OR'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/* | ||
* 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 { EuiBadge } from '@elastic/eui'; | ||
import React from 'react'; | ||
import styled from 'styled-components'; | ||
|
||
import * as i18n from './translations'; | ||
|
||
import { AndOr } from '.'; | ||
|
||
const RoundBadge = (styled(EuiBadge)` | ||
align-items: center; | ||
border-radius: 100%; | ||
display: inline-flex; | ||
font-size: 9px; | ||
height: 34px; | ||
justify-content: center; | ||
margin: 0 5px 0 5px; | ||
padding: 7px 6px 4px 6px; | ||
user-select: none; | ||
width: 34px; | ||
.euiBadge__content { | ||
position: relative; | ||
top: -1px; | ||
} | ||
.euiBadge__text { | ||
text-overflow: clip; | ||
} | ||
` as unknown) as typeof EuiBadge; | ||
|
||
RoundBadge.displayName = 'RoundBadge'; | ||
|
||
export const RoundedBadge: React.FC<{ type: AndOr }> = ({ type }) => ( | ||
<RoundBadge data-test-subj="and-or-badge" color="hollow"> | ||
{type === 'and' ? i18n.AND : i18n.OR} | ||
</RoundBadge> | ||
); | ||
|
||
RoundedBadge.displayName = 'RoundedBadge'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* 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 React from 'react'; | ||
import { ThemeProvider } from 'styled-components'; | ||
import { mount } from 'enzyme'; | ||
|
||
import { getMockTheme } from '../../../common/test_utils/kibana_react.mock'; | ||
|
||
import { RoundedBadgeAntenna } from './rounded_badge_antenna'; | ||
|
||
const mockTheme = getMockTheme({ eui: { euiColorLightShade: '#ece' } }); | ||
|
||
describe('RoundedBadgeAntenna', () => { | ||
test('it renders top and bottom antenna bars', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<RoundedBadgeAntenna type="and" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('AND'); | ||
expect(wrapper.find('[data-test-subj="andOrBadgeBarTop"]').exists()).toBeTruthy(); | ||
expect(wrapper.find('[data-test-subj="andOrBadgeBarBottom"]').exists()).toBeTruthy(); | ||
}); | ||
|
||
test('it renders "and" when "type" is "and"', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<RoundedBadgeAntenna type="and" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('AND'); | ||
}); | ||
|
||
test('it renders "or" when "type" is "or"', () => { | ||
const wrapper = mount( | ||
<ThemeProvider theme={mockTheme}> | ||
<RoundedBadgeAntenna type="or" /> | ||
</ThemeProvider> | ||
); | ||
|
||
expect(wrapper.find('[data-test-subj="and-or-badge"]').at(0).text()).toEqual('OR'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/* | ||
* 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 { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; | ||
import React from 'react'; | ||
import styled, { css } from 'styled-components'; | ||
|
||
import { RoundedBadge } from './rounded_badge'; | ||
|
||
import { AndOr } from '.'; | ||
|
||
const antennaStyles = css` | ||
background: ${({ theme }): string => theme.eui.euiColorLightShade}; | ||
position: relative; | ||
width: 2px; | ||
&:after { | ||
background: ${({ theme }): string => theme.eui.euiColorLightShade}; | ||
content: ''; | ||
height: 8px; | ||
right: -4px; | ||
position: absolute; | ||
width: 10px; | ||
clip-path: circle(); | ||
} | ||
`; | ||
|
||
const TopAntenna = styled(EuiFlexItem)` | ||
${antennaStyles} | ||
&:after { | ||
top: 0; | ||
} | ||
`; | ||
const BottomAntenna = styled(EuiFlexItem)` | ||
${antennaStyles} | ||
&:after { | ||
bottom: 0; | ||
} | ||
`; | ||
|
||
export const RoundedBadgeAntenna: React.FC<{ type: AndOr }> = ({ type }) => ( | ||
<EuiFlexGroup | ||
className="andBadgeContainer" | ||
gutterSize="none" | ||
direction="column" | ||
alignItems="center" | ||
> | ||
<TopAntenna data-test-subj="andOrBadgeBarTop" grow={1} /> | ||
<EuiFlexItem grow={false}> | ||
<RoundedBadge type={type} /> | ||
</EuiFlexItem> | ||
<BottomAntenna data-test-subj="andOrBadgeBarBottom" grow={1} /> | ||
</EuiFlexGroup> | ||
); | ||
|
||
RoundedBadgeAntenna.displayName = 'RoundedBadgeAntenna'; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?