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

[Security Solution] Remove fields from sourcerer response #130917

Merged
merged 8 commits into from
May 11, 2022
Merged

[Security Solution] Remove fields from sourcerer response #130917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
68422c9
rm data view fields from sourcerer call
stephmilovic Apr 25, 2022
f707f4e
another opt
stephmilovic Apr 25, 2022
59588ca
update tests
stephmilovic Apr 25, 2022
08ec8a9
Merge branch 'main' into rm_data_view_get
kibanamachine Apr 25, 2022
fc92e8f
Merge branch 'main' into rm_data_view_get
kibanamachine Apr 27, 2022
b73deff
rm commented out code
stephmilovic Apr 28, 2022
02c7160
Merge branch 'main' into rm_data_view_get
kibanamachine May 2, 2022
5976e8f
Merge branch 'main' into rm_data_view_get
kibanamachine May 10, 2022
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
8 changes: 7 additions & 1 deletion x-pack/plugins/security_solution/server/lib/sourcerer/routes/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ jest.mock('./helpers', () => {
});
const mockPattern = {
id: 'security-solution',
fields: [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding fields to mock response to ensure they are not returned

{ name: '@timestamp', searchable: true, type: 'date', aggregatable: true },
{ name: '@version', searchable: true, type: 'string', aggregatable: true },
{ name: 'agent.ephemeral_id', searchable: true, type: 'string', aggregatable: true },
{ name: 'agent.hostname', searchable: true, type: 'string', aggregatable: true },
{ name: 'agent.id', searchable: true, type: 'string', aggregatable: true },
],
title:
'apm-*-transaction*,traces-apm*,auditbeat-*,endgame-*,filebeat-*,logs-*,packetbeat-*,winlogbeat-*,ml_host_risk_score_*,.siem-signals-default',
};
Expand Down Expand Up @@ -147,7 +154,6 @@ describe('sourcerer route', () => {

test('returns sourcerer formatted Data Views when SIEM Data View does NOT exist but has been created in the mean time', async () => {
const getMock = jest.fn();
getMock.mockResolvedValueOnce(null);
getMock.mockResolvedValueOnce(mockPattern);
const getStartServicesSpecial = jest.fn().mockResolvedValue([
null,
Expand Down
37 changes: 22 additions & 15 deletions x-pack/plugins/security_solution/server/lib/sourcerer/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,8 @@ export const createSourcererDataViewRoute = (
);

let allDataViews: DataViewListItem[] = await dataViewService.getIdsWithTitle();
let siemDataView = null;
try {
siemDataView = await dataViewService.get(dataViewId);
} catch (err) {
const error = transformError(err);
// Do nothing if statusCode === 404 because we expect that the security dataview does not exist
if (error.statusCode !== 404) {
throw err;
}
}
let siemDataView: DataView | DataViewListItem | null =
allDataViews.find((dv) => dv.id === dataViewId) ?? null;

const { patternList } = request.body;
const patternListAsTitle = patternList.sort().join();
Expand All @@ -90,6 +82,7 @@ export const createSourcererDataViewRoute = (
}
}
} else if (patternListAsTitle !== siemDataViewTitle) {
siemDataView = await dataViewService.get(dataViewId);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can get to this line only if siemDataView is not null, so I didn't understand the logic, why we need to reassign the new value fetched from dataViewService to siemDataView, is there any purpose for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, unfortunately to make the dataViewService.updateSavedObject call we need to pass the entire data view object including fields 😢

Copy link
Contributor

Choose a reason for hiding this comment

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

@stephmilovic is this still true? From the docs it says Update part of an data view. Only the specified fields are updated in the data view. Unspecified fields stay as they are persisted.. Are we just looking to update title here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yctercero unfortunately, if we do not do the GET call we do not get the saved object methods. It's kind of silly, they should really fetch it on their end. But alas, sending only id and title to dataViewService.updateSavedObject results in

error TypeError: indexPattern.getAsSavedObjectBody is not a function
    at DataViewsService.updateSavedObject (/Users/stephmilovic/dev/kibana/src/plugins/data_views/common/data_views/data_views.ts:665:31)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the follow-up Steph! That stinks...

siemDataView.title = patternListAsTitle;
await dataViewService.updateSavedObject(siemDataView);
}
Expand Down Expand Up @@ -160,8 +153,9 @@ export const getSourcererDataViewRoute = (
request,
true
);

const siemDataView = await dataViewService.get(dataViewId);
const allDataViews: DataViewListItem[] = await dataViewService.getIdsWithTitle();
const siemDataView: DataViewListItem | null =
allDataViews.find((dv) => dv.id === dataViewId) ?? null;
const kibanaDataView = siemDataView
? await buildSourcererDataView(
siemDataView,
Expand All @@ -186,14 +180,27 @@ export const getSourcererDataViewRoute = (
);
};

interface KibanaDataView {
/** Uniquely identifies a Kibana Data View */
id: string;
/** list of active patterns that return data */
patternList: string[];
/**
* title of Kibana Data View
* title also serves as "all pattern list", including inactive
* comma separated string
*/
title: string;
}

const buildSourcererDataView = async (
dataView: DataView,
dataView: DataView | DataViewListItem,
clientAsCurrentUser: ElasticsearchClient
) => {
): Promise<KibanaDataView> => {
const patternList = dataView.title.split(',');
const activePatternBools: boolean[] = await findExistingIndices(patternList, clientAsCurrentUser);
const activePatternLists: string[] = patternList.filter(
(pattern, j, self) => self.indexOf(pattern) === j && activePatternBools[j]
);
return { ...dataView, patternList: activePatternLists };
return { id: dataView.id ?? '', title: dataView.title, patternList: activePatternLists };
};