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] Add alerts-as-data index names as alias to signals indices #119921

Merged
merged 2 commits into from
Dec 1, 2021
Merged

[Security Solution] Add alerts-as-data index names as alias to signals indices #119921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
deff086
Add alerts-as-data index names as alias to signals indices
marshallmain Nov 30, 2021
1d74418
Update jest snapshot
marshallmain Nov 30, 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
9 changes: 7 additions & 2 deletions .../server/lib/detection_engine/routes/index/__snapshots__/get_signals_template.test.ts.snap
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion .../plugins/security_solution/server/lib/detection_engine/routes/index/create_index_route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export const createDetectionIndex = async (
if (await templateNeedsUpdate({ alias: index, esClient })) {
await esClient.indices.putIndexTemplate({
name: index,
body: getSignalsTemplate(index, spaceId, aadIndexAliasName) as Record<string, unknown>,
body: getSignalsTemplate(index, aadIndexAliasName) as Record<string, unknown>,
});
}
// Check if the old legacy siem signals template exists and remove it
Expand Down
6 changes: 5 additions & 1 deletion .../security_solution/server/lib/detection_engine/routes/index/create_preview_index_route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export const createPreviewIndex = async (
) => {
const esClient = context.core.elasticsearch.client.asCurrentUser;
const index = siemClient.getPreviewIndex();
const spaceId = context.securitySolution.getSpaceId();

const indexExists = await getIndexExists(esClient, index);

Expand All @@ -70,10 +71,13 @@ export const createPreviewIndex = async (
await setPolicy(esClient, index, previewPolicy);
}

const ruleDataService = context.securitySolution.getRuleDataService();
const aadIndexAliasName = ruleDataService.getResourceName(`security.alerts-${spaceId}`);

if (await templateNeedsUpdate({ alias: index, esClient })) {
await esClient.indices.putIndexTemplate({
name: index,
body: getSignalsTemplate(index) as Record<string, unknown>,
body: getSignalsTemplate(index, aadIndexAliasName) as Record<string, unknown>,
Copy link
Member

Choose a reason for hiding this comment

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

I haven't been following the latest Rule Preview efforts closely so just curious here, but why are we setting up the preview index with aliases to the AAD index? Looks like it's getting setup with the signals' fieldAliases as well (within getSignalsTemplate). Is this for querying compatibility with existing components?

Copy link
Contributor Author

@marshallmain marshallmain Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We don't need the index alias, I just wanted to avoid having conditional logic in getSignalsTemplate and this route is going away entirely in #116374 (it never shipped in 7.16 so we can remove this route without issue). Since we definitely do need the alias for the "regular" .siem-signals template, I didn't want to make the "alias" parameter optional and then have more confusion about when it is/isn't needed.

After #116374, the preview index/template logic will be handled by the RuleDataService.

The field aliases do matter though, as attempting to write to a field that is defined as an alias in the mapping will result in an error. This means for the new AAD indices since the signal.* fields are still defined (as aliases now) we'll still have conflicts if we try to copy user's source data into those fields.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, perfect -- thanks for explaining! 🙂

});
}

Expand Down
42 changes: 7 additions & 35 deletions ...s/security_solution/server/lib/detection_engine/routes/index/get_signals_template.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@ import { createBackwardsCompatibilityMapping, getSignalsTemplate } from './get_s

describe('get_signals_template', () => {
test('it should set the lifecycle "name" and "rollover_alias" to be the name of the index passed in', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');
expect(template.template.settings).toEqual({
index: {
lifecycle: {
Expand All @@ -28,38 +24,22 @@ describe('get_signals_template', () => {
});

test('it should set have the index patterns with an ending glob in it', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');
expect(template.index_patterns).toEqual(['test-index-*']);
});

test('it should have a mappings section which is an object type', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');
expect(typeof template.template.mappings).toEqual('object');
});

test('it should have a signals section which is an object type', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');
expect(typeof template.template.mappings.properties.signal).toEqual('object');
});

test('it should have a "total_fields" section that is at least 10k in size', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');
expect(template.template.settings.mapping.total_fields.limit).toBeGreaterThanOrEqual(10000);
});

Expand All @@ -82,11 +62,7 @@ describe('get_signals_template', () => {
// Instead you have to use "keyword". This test was first introduced when ECS 1.10 came out and data_stream.* values which had
// "constant_keyword" fields and we needed to change those to be "keyword" instead.
test('it should NOT have any "constant_keyword" and instead those should be replaced with regular "keyword" in the mapping', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');

// Small recursive function to find any values of "constant_keyword" and mark which fields it was found on and then error on those fields
// The matchers from jest such as jest.toMatchObject do not support recursion, so I have to write it here:
Expand Down Expand Up @@ -117,11 +93,7 @@ describe('get_signals_template', () => {
});

test('it should match snapshot', () => {
const template = getSignalsTemplate(
'test-index',
'space-id',
'.alerts-security.alerts-space-id'
);
const template = getSignalsTemplate('test-index', '.alerts-security.alerts-space-id');
expect(template).toMatchSnapshot();
});

Expand Down
16 changes: 8 additions & 8 deletions ...lugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import signalExtraFields from './signal_extra_fields.json';
incremented by 10 in order to add "room" for the aforementioned patch
release
*/
export const SIGNALS_TEMPLATE_VERSION = 57;
export const SIGNALS_TEMPLATE_VERSION = 67;
/**
@constant
@type {number}
Expand All @@ -59,16 +59,16 @@ export const SIGNALS_FIELD_ALIASES_VERSION = 1;
export const MIN_EQL_RULE_INDEX_VERSION = 2;
export const ALIAS_VERSION_FIELD = 'aliases_version';

export const getSignalsTemplate = (index: string, spaceId?: string, aadIndexAliasName?: string) => {
export const getSignalsTemplate = (index: string, aadIndexAliasName: string) => {
const fieldAliases = createSignalsFieldAliases();
const template = {
index_patterns: [`${index}-*`],
template: {
// aliases: {
// [aadIndexAliasName]: {
// is_write_index: false,
// },
// },
aliases: {
[aadIndexAliasName]: {
is_write_index: false,
},
},
settings: {
index: {
lifecycle: {
Expand Down Expand Up @@ -142,7 +142,7 @@ const properties = {
export const backwardsCompatibilityMappings = [
{
minVersion: 0,
// Version 45 shipped with 7.14
// Version 45 shipped with 7.14. 7.15+ have both the host.os.name.caseless field and the field aliases
maxVersion: 45,
mapping: {
runtime: {
Expand Down