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

Jump to bottom

[EPM] Conditionally generate ES index pattern name based on dataset_is_prefix #89870

Merged
merged 10 commits into from
Feb 8, 2021
Merged

[EPM] Conditionally generate ES index pattern name based on dataset_is_prefix #89870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9fb8dfa
Explicitly generate ES index pattern name.
skh Feb 1, 2021
0d45453
Adjust tests.
skh Feb 1, 2021
f5b4dcc
Adjust and reenable tests.
skh Feb 2, 2021
0642673
Set template priority based on dataset_is_prefix
skh Feb 3, 2021
764b414
Refactor indexPatternName -> templateIndexPattern
skh Feb 3, 2021
47f7556
Add unit tests.
skh Feb 5, 2021
890e2de
Use more realistic index pattern in test.
skh Feb 5, 2021
4e543cb
Fix unit test.
skh Feb 5, 2021
0367454
Add unit test for installTemplate().
skh Feb 8, 2021
c7887ba
Merge branch 'master' into 88307-use-dataset-is-prefix
kibanamachine Feb 8, 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
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/types/models/epm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export interface RegistryDataStream {
path: string;
ingest_pipeline: string;
elasticsearch?: RegistryElasticsearch;
dataset_is_prefix?: boolean;
}

export interface RegistryElasticsearch {
Expand Down
110 changes: 110 additions & 0 deletions x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* 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 { RegistryDataStream } from '../../../../types';
import { Field } from '../../fields/field';

import { elasticsearchServiceMock } from 'src/core/server/mocks';
import { installTemplate } from './install';

test('tests installPackage to use correct priority and index_patterns for data stream with dataset_is_prefix not set', async () => {
const callCluster = elasticsearchServiceMock.createLegacyScopedClusterClient().callAsCurrentUser;
const fields: Field[] = [];
const dataStreamDatasetIsPrefixUnset = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
} as RegistryDataStream;
const pkg = {
name: 'package',
version: '0.0.1',
};
const templateIndexPatternDatasetIsPrefixUnset = 'metrics-package.dataset-*';
const templatePriorityDatasetIsPrefixUnset = 200;
await installTemplate({
callCluster,
fields,
dataStream: dataStreamDatasetIsPrefixUnset,
packageVersion: pkg.version,
packageName: pkg.name,
});
// @ts-ignore
const sentTemplate = callCluster.mock.calls[0][1].body;
expect(sentTemplate).toBeDefined();
expect(sentTemplate.priority).toBe(templatePriorityDatasetIsPrefixUnset);
expect(sentTemplate.index_patterns).toEqual([templateIndexPatternDatasetIsPrefixUnset]);
});

test('tests installPackage to use correct priority and index_patterns for data stream with dataset_is_prefix set to false', async () => {
const callCluster = elasticsearchServiceMock.createLegacyScopedClusterClient().callAsCurrentUser;
const fields: Field[] = [];
const dataStreamDatasetIsPrefixFalse = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
dataset_is_prefix: false,
} as RegistryDataStream;
const pkg = {
name: 'package',
version: '0.0.1',
};
const templateIndexPatternDatasetIsPrefixFalse = 'metrics-package.dataset-*';
const templatePriorityDatasetIsPrefixFalse = 200;
await installTemplate({
callCluster,
fields,
dataStream: dataStreamDatasetIsPrefixFalse,
packageVersion: pkg.version,
packageName: pkg.name,
});
// @ts-ignore
const sentTemplate = callCluster.mock.calls[0][1].body;
expect(sentTemplate).toBeDefined();
expect(sentTemplate.priority).toBe(templatePriorityDatasetIsPrefixFalse);
expect(sentTemplate.index_patterns).toEqual([templateIndexPatternDatasetIsPrefixFalse]);
});

test('tests installPackage to use correct priority and index_patterns for data stream with dataset_is_prefix set to true', async () => {
const callCluster = elasticsearchServiceMock.createLegacyScopedClusterClient().callAsCurrentUser;
const fields: Field[] = [];
const dataStreamDatasetIsPrefixTrue = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
dataset_is_prefix: true,
} as RegistryDataStream;
const pkg = {
name: 'package',
version: '0.0.1',
};
const templateIndexPatternDatasetIsPrefixTrue = 'metrics-package.dataset.*-*';
const templatePriorityDatasetIsPrefixTrue = 150;
await installTemplate({
callCluster,
fields,
dataStream: dataStreamDatasetIsPrefixTrue,
packageVersion: pkg.version,
packageName: pkg.name,
});
// @ts-ignore
const sentTemplate = callCluster.mock.calls[0][1].body;
expect(sentTemplate).toBeDefined();
expect(sentTemplate.priority).toBe(templatePriorityDatasetIsPrefixTrue);
expect(sentTemplate.index_patterns).toEqual([templateIndexPatternDatasetIsPrefixTrue]);
});
14 changes: 12 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ import {
import { CallESAsCurrentUser } from '../../../../types';
import { Field, loadFieldsFromYaml, processFields } from '../../fields/field';
import { getPipelineNameForInstallation } from '../ingest_pipeline/install';
import { generateMappings, generateTemplateName, getTemplate } from './template';
import {
generateMappings,
generateTemplateName,
generateTemplateIndexPattern,
getTemplate,
getTemplatePriority,
} from './template';
import { getAsset, getPathParts } from '../../archive';
import { removeAssetsFromInstalledEsByType, saveInstalledEsRefs } from '../../packages/install';

Expand Down Expand Up @@ -293,6 +299,9 @@ export async function installTemplate({
}): Promise<TemplateRef> {
const mappings = generateMappings(processFields(fields));
const templateName = generateTemplateName(dataStream);
const templateIndexPattern = generateTemplateIndexPattern(dataStream);
const templatePriority = getTemplatePriority(dataStream);

let pipelineName;
if (dataStream.ingest_pipeline) {
pipelineName = getPipelineNameForInstallation({
Expand All @@ -310,11 +319,12 @@ export async function installTemplate({

const template = getTemplate({
type: dataStream.type,
templateName,
templateIndexPattern,
mappings,
pipelineName,
packageName,
composedOfTemplates,
templatePriority,
ilmPolicy: dataStream.ilm_policy,
hidden: dataStream.hidden,
});
Expand Down
99 changes: 85 additions & 14 deletions x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@
import { readFileSync } from 'fs';
import { safeLoad } from 'js-yaml';
import path from 'path';
import { RegistryDataStream } from '../../../../types';
import { Field, processFields } from '../../fields/field';
import { generateMappings, getTemplate } from './template';
import {
generateMappings,
getTemplate,
getTemplatePriority,
generateTemplateIndexPattern,
} from './template';

// Add our own serialiser to just do JSON.stringify
expect.addSnapshotSerializer({
Expand All @@ -23,27 +29,29 @@ expect.addSnapshotSerializer({
});

test('get template', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be great to have a unit test for installTemplate since that's the method that generates the index pattern and priority based on data stream information. I don't see any tests here that actually test on the dataset_is_prefix property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Is there an established way how to mock calls to elasticsearch in unit tests?

I've added unit tests for generateTemplateIndexPattern() and getTemplatePriority() as a start in 8b58f46

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like Kibana core does export some ES mocks, here's an example: https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/services/epm/packages/_install_package.test.ts#L38

I see that installTemplate passes callCluster to other methods, so I guess you might have to mock ES return values in order of whenever callCluster is called further down the chain. the mocked ES client returns a jest.fn() so mocking the responses could be done like this:

const mockCallCluster = elasticsearchServiceMock.createLegacyScopedClusterClient().callAsCurrentUser
  .mockReturnValueOnce({ success: true })
  .mockReturnValueOnce({ success: true })
  .mockReturnValueOnce(// change mocked response whenever the response matters)
  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the pointer, I've added a unit test for installTemplate().

const templateName = 'logs-nginx-access-abcd';
const templateIndexPattern = 'logs-nginx.access-abcd-*';

const template = getTemplate({
type: 'logs',
templateName,
templateIndexPattern,
packageName: 'nginx',
mappings: { properties: {} },
composedOfTemplates: [],
templatePriority: 200,
});
expect(template.index_patterns).toStrictEqual([`${templateName}-*`]);
expect(template.index_patterns).toStrictEqual([templateIndexPattern]);
});

test('adds composed_of correctly', () => {
const composedOfTemplates = ['component1', 'component2'];

const template = getTemplate({
type: 'logs',
templateName: 'name',
templateIndexPattern: 'name-*',
packageName: 'nginx',
mappings: { properties: {} },
composedOfTemplates,
templatePriority: 200,
});
expect(template.composed_of).toStrictEqual(composedOfTemplates);
});
Expand All @@ -53,35 +61,36 @@ test('adds empty composed_of correctly', () => {

const template = getTemplate({
type: 'logs',
templateName: 'name',
templateIndexPattern: 'name-*',
packageName: 'nginx',
mappings: { properties: {} },
composedOfTemplates,
templatePriority: 200,
});
expect(template.composed_of).toStrictEqual(composedOfTemplates);
});

test('adds hidden field correctly', () => {
const templateWithHiddenName = 'logs-nginx-access-abcd';
const templateIndexPattern = 'logs-nginx.access-abcd-*';

const templateWithHidden = getTemplate({
type: 'logs',
templateName: templateWithHiddenName,
templateIndexPattern,
packageName: 'nginx',
mappings: { properties: {} },
composedOfTemplates: [],
templatePriority: 200,
hidden: true,
});
expect(templateWithHidden.data_stream.hidden).toEqual(true);

const templateWithoutHiddenName = 'logs-nginx-access-efgh';

const templateWithoutHidden = getTemplate({
type: 'logs',
templateName: templateWithoutHiddenName,
templateIndexPattern,
packageName: 'nginx',
mappings: { properties: {} },
composedOfTemplates: [],
templatePriority: 200,
});
expect(templateWithoutHidden.data_stream.hidden).toEqual(undefined);
});
Expand All @@ -95,10 +104,11 @@ test('tests loading base.yml', () => {
const mappings = generateMappings(processedFields);
const template = getTemplate({
type: 'logs',
templateName: 'foo',
templateIndexPattern: 'foo-*',
packageName: 'nginx',
mappings,
composedOfTemplates: [],
templatePriority: 200,
});

expect(template).toMatchSnapshot(path.basename(ymlPath));
Expand All @@ -113,10 +123,11 @@ test('tests loading coredns.logs.yml', () => {
const mappings = generateMappings(processedFields);
const template = getTemplate({
type: 'logs',
templateName: 'foo',
templateIndexPattern: 'foo-*',
packageName: 'coredns',
mappings,
composedOfTemplates: [],
templatePriority: 200,
});

expect(template).toMatchSnapshot(path.basename(ymlPath));
Expand All @@ -131,10 +142,11 @@ test('tests loading system.yml', () => {
const mappings = generateMappings(processedFields);
const template = getTemplate({
type: 'metrics',
templateName: 'whatsthis',
templateIndexPattern: 'whatsthis-*',
packageName: 'system',
mappings,
composedOfTemplates: [],
templatePriority: 200,
});

expect(template).toMatchSnapshot(path.basename(ymlPath));
Expand Down Expand Up @@ -520,3 +532,62 @@ test('tests constant_keyword field type handling', () => {
const mappings = generateMappings(processedFields);
expect(JSON.stringify(mappings)).toEqual(JSON.stringify(constantKeywordMapping));
});

test('tests priority and index pattern for data stream without dataset_is_prefix', () => {
const dataStreamDatasetIsPrefixUnset = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
} as RegistryDataStream;
const templateIndexPatternDatasetIsPrefixUnset = 'metrics-package.dataset-*';
const templatePriorityDatasetIsPrefixUnset = 200;
const templateIndexPattern = generateTemplateIndexPattern(dataStreamDatasetIsPrefixUnset);
const templatePriority = getTemplatePriority(dataStreamDatasetIsPrefixUnset);

expect(templateIndexPattern).toEqual(templateIndexPatternDatasetIsPrefixUnset);
expect(templatePriority).toEqual(templatePriorityDatasetIsPrefixUnset);
});

test('tests priority and index pattern for data stream with dataset_is_prefix set to false', () => {
const dataStreamDatasetIsPrefixFalse = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
dataset_is_prefix: false,
} as RegistryDataStream;
const templateIndexPatternDatasetIsPrefixFalse = 'metrics-package.dataset-*';
const templatePriorityDatasetIsPrefixFalse = 200;
const templateIndexPattern = generateTemplateIndexPattern(dataStreamDatasetIsPrefixFalse);
const templatePriority = getTemplatePriority(dataStreamDatasetIsPrefixFalse);

expect(templateIndexPattern).toEqual(templateIndexPatternDatasetIsPrefixFalse);
expect(templatePriority).toEqual(templatePriorityDatasetIsPrefixFalse);
});

test('tests priority and index pattern for data stream with dataset_is_prefix set to true', () => {
const dataStreamDatasetIsPrefixTrue = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
dataset_is_prefix: true,
} as RegistryDataStream;
const templateIndexPatternDatasetIsPrefixTrue = 'metrics-package.dataset.*-*';
const templatePriorityDatasetIsPrefixTrue = 150;
const templateIndexPattern = generateTemplateIndexPattern(dataStreamDatasetIsPrefixTrue);
const templatePriority = getTemplatePriority(dataStreamDatasetIsPrefixTrue);

expect(templateIndexPattern).toEqual(templateIndexPatternDatasetIsPrefixTrue);
expect(templatePriority).toEqual(templatePriorityDatasetIsPrefixTrue);
});
Loading