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.

Sign up for GitHub

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

[SIEM] Implement NP Plugin Setup #54030

Merged
merged 21 commits into from
Jan 7, 2020
Merged

[SIEM] Implement NP Plugin Setup #54030

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
eabb52d
Set up our react app in the NP way
rylnd Dec 19, 2019
d715578
Move files into NP structure
rylnd Dec 19, 2019
f9e0c53
Register SIEM in the feature catalogue via NP format
rylnd Dec 20, 2019
ad71223
Eliminate usage of timezoneBrowser UI setting
rylnd Jan 2, 2020
25c379c
Clean up tests for FormattedDate components
rylnd Jan 3, 2020
03cd2c1
Remove remaining uses of UI Settings mocks
rylnd Jan 3, 2020
6c74a91
Remove unnecessary intermediate `describe` blocks
rylnd Jan 3, 2020
b4b4635
Remove use of kibana version in client requests
rylnd Jan 6, 2020
896d7e0
Remove newly added uses of kbnVersion
rylnd Jan 6, 2020
256479f
Use helper to generate test assertion
rylnd Jan 6, 2020
afd5f17
Remove guard from date formatting component
rylnd Jan 7, 2020
215f59c
Remove default from byte formatting component
rylnd Jan 7, 2020
e071875
Refactor bytes formatting to allow use in our charts
rylnd Jan 7, 2020
7f971f7
Refer to our constant for APP_ID
rylnd Jan 7, 2020
0d923e8
Explicit return values for some UI Settings hooks
rylnd Jan 7, 2020
ae70785
Remove use of ui/chrome in request header
rylnd Jan 7, 2020
1a8a8eb
Mock UI Settings values in our TestProvider
rylnd Jan 7, 2020
69dbfae
Remove unused import
rylnd Jan 7, 2020
75ffe9b
Merge branch 'master' into siem_ui_setup
rylnd Jan 7, 2020
06d5807
Merge branch 'master' into siem_ui_setup
rylnd Jan 7, 2020
c01e51d
Merge branch 'master' into siem_ui_setup
elasticmachine Jan 7, 2020
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
2 changes: 0 additions & 2 deletions x-pack/legacy/plugins/siem/common/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ export const DEFAULT_SEARCH_AFTER_PAGE_SIZE = 100;
export const DEFAULT_ANOMALY_SCORE = 'siem:defaultAnomalyScore';
export const DEFAULT_MAX_TABLE_QUERY_SIZE = 10000;
export const DEFAULT_SCALE_DATE_FORMAT = 'dateFormat:scaled';
export const DEFAULT_KBN_VERSION = 'kbnVersion';
export const DEFAULT_TIMEZONE_BROWSER = 'timezoneBrowser';
export const DEFAULT_FROM = 'now-24h';
export const DEFAULT_TO = 'now';
export const DEFAULT_INTERVAL_PAUSE = true;
Expand Down
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/siem/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { resolve } from 'path';
import { Server } from 'hapi';
import { Root } from 'joi';

import { PluginInitializerContext } from 'src/core/server';
import { PluginInitializerContext } from '../../../../src/core/server';
import { plugin } from './server';
import { savedObjectMappings } from './server/saved_objects';

Expand Down Expand Up @@ -43,7 +43,7 @@ export const siem = (kibana: any) => {
description: i18n.translate('xpack.siem.securityDescription', {
defaultMessage: 'Explore your SIEM App',
}),
main: 'plugins/siem/app',
main: 'plugins/siem/legacy',
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, this was mimicked from:
https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/infra/index.ts#L36

Looking at others though they have legacy in their name now such as:
https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/maps/index.js

So this looks 👍 to me.

euiIconType: 'securityAnalyticsApp',
title: APP_NAME,
listed: false,
Expand Down
6 changes: 2 additions & 4 deletions ...cy/plugins/siem/public/apps/start_app.tsx → ...ck/legacy/plugins/siem/public/app/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { DEFAULT_DARK_MODE } from '../../common/constants';
import { ErrorToastDispatcher } from '../components/error_toast_dispatcher';
import { compose } from '../lib/compose/kibana_compose';
import { AppFrontendLibs } from '../lib/lib';
import { StartCore, StartPlugins } from './plugin';
import { CoreStart, StartPlugins } from '../plugin';
import { PageRouter } from '../routes';
import { createStore } from '../store';
import { GlobalToaster, ManageGlobalToaster } from '../components/toasters';
Expand Down Expand Up @@ -71,9 +71,7 @@ const StartApp: FC<AppFrontendLibs> = memo(libs => {
return <AppPluginRoot />;
});

export const ROOT_ELEMENT_ID = 'react-siem-root';

export const SiemApp = memo<{ core: StartCore; plugins: StartPlugins }>(({ core, plugins }) => (
export const SiemApp = memo<{ core: CoreStart; plugins: StartPlugins }>(({ core, plugins }) => (
<KibanaContextProvider
services={{
appName: 'siem',
Expand Down
20 changes: 20 additions & 0 deletions x-pack/legacy/plugins/siem/public/app/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';

import { CoreStart, StartPlugins, AppMountParameters } from '../plugin';
import { SiemApp } from './app';

export const renderApp = (
core: CoreStart,
plugins: StartPlugins,
{ element }: AppMountParameters
) => {
render(<SiemApp core={core} plugins={plugins} />, element);
return () => unmountComponentAtNode(element);
};
18 changes: 0 additions & 18 deletions x-pack/legacy/plugins/siem/public/apps/index.ts
View file
Open in desktop

This file was deleted.

59 changes: 0 additions & 59 deletions x-pack/legacy/plugins/siem/public/apps/plugin.tsx
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion x-pack/legacy/plugins/siem/public/apps/template.html
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/siem/public/components/charts/areachart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { getOr, get, isNull, isNumber } from 'lodash/fp';
import { AutoSizer } from '../auto_sizer';
import { ChartPlaceHolder } from './chart_place_holder';
import { useTimeZone } from '../../hooks';
import {
chartDefaultSettings,
ChartSeriesConfigs,
Expand All @@ -26,7 +27,6 @@ import {
getChartWidth,
WrappedByAutoSizer,
useTheme,
useBrowserTimeZone,
} from './common';

// custom series styles: https://ela.st/areachart-styling
Expand Down Expand Up @@ -71,7 +71,7 @@ export const AreaChartBaseComponent = ({
configs?: ChartSeriesConfigs | undefined;
}) => {
const theme = useTheme();
const timeZone = useBrowserTimeZone();
const timeZone = useTimeZone();
const xTickFormatter = get('configs.axis.xTickFormatter', chartConfigs);
const yTickFormatter = get('configs.axis.yTickFormatter', chartConfigs);
const xAxisId = `group-${data[0].key}-x`;
Expand Down
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/siem/public/components/charts/barchart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import React from 'react';
import { Chart, BarSeries, Axis, Position, ScaleType, Settings } from '@elastic/charts';
import { getOr, get, isNumber } from 'lodash/fp';
import { useTimeZone } from '../../hooks';
import { AutoSizer } from '../auto_sizer';
import { ChartPlaceHolder } from './chart_place_holder';
import {
Expand All @@ -17,7 +18,6 @@ import {
getChartHeight,
getChartWidth,
WrappedByAutoSizer,
useBrowserTimeZone,
useTheme,
} from './common';

Expand All @@ -44,7 +44,7 @@ export const BarChartBaseComponent = ({
configs?: ChartSeriesConfigs | undefined;
}) => {
const theme = useTheme();
const timeZone = useBrowserTimeZone();
const timeZone = useTimeZone();
const xTickFormatter = get('configs.axis.xTickFormatter', chartConfigs);
const yTickFormatter = get('configs.axis.yTickFormatter', chartConfigs);
const tickSize = getOr(0, 'configs.axis.tickSize', chartConfigs);
Expand Down
8 changes: 1 addition & 7 deletions x-pack/legacy/plugins/siem/public/components/charts/common.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ import {
SettingsSpecProps,
TickFormatter,
} from '@elastic/charts';
import moment from 'moment-timezone';
import styled from 'styled-components';
import { useUiSetting } from '../../lib/kibana';
import { DEFAULT_DATE_FORMAT_TZ, DEFAULT_DARK_MODE } from '../../../common/constants';
import { DEFAULT_DARK_MODE } from '../../../common/constants';

export const defaultChartHeight = '100%';
export const defaultChartWidth = '100%';
Expand Down Expand Up @@ -108,11 +107,6 @@ export const chartDefaultSettings = {
debug: false,
};

export const useBrowserTimeZone = () => {
const kibanaTimezone = useUiSetting<string>(DEFAULT_DATE_FORMAT_TZ);
return kibanaTimezone === 'Browser' ? moment.tz.guess() : kibanaTimezone;
};

export const getChartHeight = (customHeight?: number, autoSizerHeight?: number): string => {
const height = customHeight || autoSizerHeight;
return height ? `${height}px` : defaultChartHeight;
Expand Down
2 changes: 1 addition & 1 deletion ...k/legacy/plugins/siem/public/components/formatted_bytes/__snapshots__/index.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

70 changes: 30 additions & 40 deletions x-pack/legacy/plugins/siem/public/components/formatted_bytes/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,58 +8,48 @@ import { mount, shallow } from 'enzyme';
import toJson from 'enzyme-to-json';
import * as React from 'react';

import { mockFrameworks, getMockKibanaUiSetting } from '../../mock';
import { useUiSetting$ } from '../../lib/kibana';

import { PreferenceFormattedBytesComponent } from '.';

jest.mock('../../lib/kibana');
const mockUseUiSetting$ = useUiSetting$ as jest.Mock;

describe('formatted_bytes', () => {
describe('PreferenceFormattedBytes', () => {
describe('rendering', () => {
beforeEach(() => {
mockUseUiSetting$.mockClear();
});
const bytes = '2806422';

const bytes = '2806422';
describe('PreferenceFormattedBytes', () => {
test('renders correctly against snapshot', () => {
mockUseUiSetting$.mockImplementation(() => ['0,0.[0]b']);
const wrapper = shallow(<PreferenceFormattedBytesComponent value={bytes} />);

test('renders correctly against snapshot', () => {
mockUseUiSetting$.mockImplementation(
getMockKibanaUiSetting(mockFrameworks.default_browser)
);
const wrapper = shallow(<PreferenceFormattedBytesComponent value={bytes} />);
expect(toJson(wrapper)).toMatchSnapshot();
});
expect(toJson(wrapper)).toMatchSnapshot();
});

test('it renders bytes to hardcoded format when no configuration exists', () => {
mockUseUiSetting$.mockImplementation(() => [null]);
const wrapper = mount(<PreferenceFormattedBytesComponent value={bytes} />);

expect(wrapper.text()).toEqual('2.7MB');
});

test('it renders bytes to hardcoded format when no configuration exists', () => {
mockUseUiSetting$.mockImplementation(() => [null]);
const wrapper = mount(<PreferenceFormattedBytesComponent value={bytes} />);
expect(wrapper.text()).toEqual('2.7MB');
});
test('it renders bytes according to the default format', () => {
mockUseUiSetting$.mockImplementation(() => ['0,0.[0]b']);
const wrapper = mount(<PreferenceFormattedBytesComponent value={bytes} />);

test('it renders bytes according to the default format', () => {
mockUseUiSetting$.mockImplementation(
getMockKibanaUiSetting(mockFrameworks.default_browser)
);
const wrapper = mount(<PreferenceFormattedBytesComponent value={bytes} />);
expect(wrapper.text()).toEqual('2.7MB');
});
expect(wrapper.text()).toEqual('2.7MB');
});

test('it renders bytes supplied as a number according to the default format', () => {
mockUseUiSetting$.mockImplementation(() => ['0,0.[0]b']);
const wrapper = mount(<PreferenceFormattedBytesComponent value={+bytes} />);

expect(wrapper.text()).toEqual('2.7MB');
});

test('it renders bytes supplied as a number according to the default format', () => {
mockUseUiSetting$.mockImplementation(
getMockKibanaUiSetting(mockFrameworks.default_browser)
);
const wrapper = mount(<PreferenceFormattedBytesComponent value={+bytes} />);
expect(wrapper.text()).toEqual('2.7MB');
});
test('it renders bytes according to new format', () => {
mockUseUiSetting$.mockImplementation(() => ['0b']);
const wrapper = mount(<PreferenceFormattedBytesComponent value={bytes} />);

test('it renders bytes according to new format', () => {
mockUseUiSetting$.mockImplementation(getMockKibanaUiSetting(mockFrameworks.bytes_short));
const wrapper = mount(<PreferenceFormattedBytesComponent value={bytes} />);
expect(wrapper.text()).toEqual('3MB');
});
});
expect(wrapper.text()).toEqual('3MB');
});
});
4 changes: 1 addition & 3 deletions x-pack/legacy/plugins/siem/public/components/formatted_bytes/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import { useUiSetting$ } from '../../lib/kibana';

export const PreferenceFormattedBytesComponent = ({ value }: { value: string | number }) => {
const [bytesFormat] = useUiSetting$<string>(DEFAULT_BYTES_FORMAT);
return (
<>{bytesFormat ? numeral(value).format(bytesFormat) : numeral(value).format('0,0.[0]b')}</>
);
return <>{numeral(value).format(bytesFormat || '0,0.[0]b')}</>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove the double pipe or check || as it looks like it is not needed since you are passing in DEFAULT_BYTES_FORMAT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we don't ever check for null/undefined in other places:
https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals/signals_utility_bar/index.tsx#L48

So I am assuming it is safe to remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an edge case where the user has explicitly unset that setting: they would get raw values with no (manual) formatting. In practice this doesn't look great, but it's not terrible either:

Network_-_Kibana

I can totally get behind the 'do what I specify in the settings' behavior and remove that guard, but I wanted to mention this just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

215f59c

Copy link
Contributor

Choose a reason for hiding this comment

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

nit, can we create a const for '0,0.[0]b' since we are also using it there https://github.com/elastic/kibana/pull/54030/files#diff-5839ee2c63a46315927b26809c783ec7R22

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@XavierM I think we're actually going to remove this particular usage. I just noticed we're using it here as well, which I'm going to inline to pull from UI settings as well. (cc @angorayc)

That just leaves references in the tests. I'm happy to declare a const DEFAULT_BYTES_FORMAT_VALUE = '0,0.[0]b' at the top of the test file for clarity. Does that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@XavierM @angorayc I refactored the byte formatting for our DNS Histogram to use the UI Setting rather than a hardcoded format: e071875. I don't have data populating in that chart, so I'm not able to test it out locally, but let me know if there are any issues there.

};

PreferenceFormattedBytesComponent.displayName = 'PreferenceFormattedBytesComponent';
Expand Down
2 changes: 1 addition & 1 deletion ...ck/legacy/plugins/siem/public/components/formatted_date/__snapshots__/index.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading