Skip to content

Commit

Permalink
[Fleet] Fix datastream settings on package upgrade and package policy…
Browse files Browse the repository at this point in the history
… creation (elastic#147368)
  • Loading branch information
nchaulet authored and nreese committed Dec 15, 2022
1 parent d941d8e commit 4dc3f94
Show file tree
Hide file tree
Showing 9 changed files with 190 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,50 @@ describe('Fleet - packageToPackagePolicy', () => {
});
});

it('returns package policy with experimental datastream features', () => {
expect(
packageToPackagePolicy(
{
...mockPackage,
savedObject: {
attributes: {
experimental_data_stream_features: [
{
data_stream: 'metrics-test.testdataset',
features: {
synthetic_source: true,
tsdb: true,
},
},
],
},
} as any,
},
'1'
)
).toEqual({
policy_id: '1',
namespace: '',
enabled: true,
inputs: [],
name: 'mock-package-1',
package: {
name: 'mock-package',
title: 'Mock package',
version: '0.0.0',
experimental_data_stream_features: [
{
data_stream: 'metrics-test.testdataset',
features: {
synthetic_source: true,
tsdb: true,
},
},
],
},
});
});

it('returns package policy with custom name', () => {
expect(packageToPackagePolicy(mockPackage, '1', 'default', 'pkgPolicy-1')).toEqual({
policy_id: '1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ export const packageToPackagePolicy = (
description?: string,
integrationToEnable?: string
): NewPackagePolicy => {
const experimentalDataStreamFeatures =
'savedObject' in packageInfo
? packageInfo.savedObject?.attributes?.experimental_data_stream_features
: undefined;

const packagePolicy: NewPackagePolicy = {
name: packagePolicyName || `${packageInfo.name}-1`,
namespace,
Expand All @@ -213,6 +218,9 @@ export const packageToPackagePolicy = (
name: packageInfo.name,
title: packageInfo.title,
version: packageInfo.version,
...(experimentalDataStreamFeatures
? { experimental_data_stream_features: experimentalDataStreamFeatures }
: undefined),
},
enabled: true,
policy_id: agentPolicyId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React from 'react';
import { act, fireEvent } from '@testing-library/react';

import { createFleetTestRendererMock } from '../../../../../../../../mock';

Expand Down Expand Up @@ -131,5 +132,55 @@ describe('ExperimentDatastreamSettings', () => {
expect(syntheticSourceSwitch).not.toBeEnabled();
expect(mockSetNewExperimentalDataFeatures).not.toBeCalled();
});

it('should not mutate original experimental feature if a user change one', () => {
const mockSetNewExperimentalDataFeatures = jest.fn();
const experimentalDataFeatures = [
{
data_stream: 'logs-test',
features: {
synthetic_source: true,
tsdb: false,
},
},
];
const res = createFleetTestRendererMock().render(
<ExperimentDatastreamSettings
registryDataStream={
{
type: 'logs',
dataset: 'test',
} as unknown as RegistryDataStream
}
experimentalDataFeatures={experimentalDataFeatures}
setNewExperimentalDataFeatures={mockSetNewExperimentalDataFeatures}
/>
);

act(() => {
fireEvent.click(
res.getByTestId('packagePolicyEditor.syntheticSourceExperimentalFeature.switch')
);
});
expect(mockSetNewExperimentalDataFeatures).toBeCalled();
expect(mockSetNewExperimentalDataFeatures.mock.calls[0][0]).toEqual([
{
data_stream: 'logs-test',
features: {
synthetic_source: false,
tsdb: false,
},
},
]);
expect(experimentalDataFeatures).toEqual([
{
data_stream: 'logs-test',
features: {
synthetic_source: true,
tsdb: false,
},
},
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ export const ExperimentDatastreamSettings: React.FunctionComponent<Props> = ({
const onIndexingSettingChange = (
features: Partial<Record<ExperimentalIndexingFeature, boolean>>
) => {
const newExperimentalDataStreamFeatures = [...(experimentalDataFeatures ?? [])];
const newExperimentalDataStreamFeatures =
experimentalDataFeatures?.map((feature) => ({ ...feature })) ?? [];

const dataStream = getRegistryDataStreamAssetBaseName(registryDataStream);

const existingSettingRecord = newExperimentalDataStreamFeatures.find(
(x) => x.data_stream === dataStream
);

if (existingSettingRecord) {
existingSettingRecord.features = {
...existingSettingRecord.features,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,44 @@ describe('EPM index template install', () => {
expect(packageTemplate.mappings).toHaveProperty('_source');
expect(packageTemplate.mappings._source).toEqual({ mode: 'synthetic' });
});

it('tests prepareTemplate to not set source mode to synthetics if specified but user disabled it', async () => {
const dataStreamDatasetIsPrefixTrue = {
type: 'metrics',
dataset: 'package.dataset',
title: 'test data stream',
release: 'experimental',
package: 'package',
path: 'path',
ingest_pipeline: 'default',
dataset_is_prefix: true,
elasticsearch: {
source_mode: 'synthetic',
},
} as RegistryDataStream;
const pkg = {
name: 'package',
version: '0.0.1',
};

const { componentTemplates } = prepareTemplate({
pkg,
dataStream: dataStreamDatasetIsPrefixTrue,
experimentalDataStreamFeature: {
data_stream: 'metrics-package.dataset',
features: {
synthetic_source: false,
tsdb: false,
},
},
});

const packageTemplate = componentTemplates['metrics-package.dataset@package'].template;

if (!('mappings' in packageTemplate)) {
throw new Error('no mappings on package template');
}

expect(packageTemplate.mappings).not.toHaveProperty('_source');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
getPipelineNameForDatastream,
getFileDataIndexName,
getFileMetadataIndexName,
getRegistryDataStreamAssetBaseName,
} from '../../../../../common/services';
import type {
RegistryDataStream,
Expand All @@ -35,6 +36,7 @@ import type {
TemplateMap,
EsAssetReference,
PackageInfo,
ExperimentalDataStreamFeature,
} from '../../../../types';
import { loadFieldsFromYaml, processFields } from '../../fields/field';
import { getAsset, getPathParts } from '../../archive';
Expand All @@ -61,7 +63,8 @@ const FLEET_COMPONENT_TEMPLATE_NAMES = FLEET_COMPONENT_TEMPLATES.map((tmpl) => t
export const prepareToInstallTemplates = (
installablePackage: InstallablePackage,
paths: string[],
esReferences: EsAssetReference[]
esReferences: EsAssetReference[],
experimentalDataStreamFeatures: ExperimentalDataStreamFeature[] = []
): {
assetsToAdd: EsAssetReference[];
assetsToRemove: EsAssetReference[];
Expand All @@ -78,9 +81,15 @@ export const prepareToInstallTemplates = (
const dataStreams = installablePackage.data_streams;
if (!dataStreams) return { assetsToAdd: [], assetsToRemove, install: () => Promise.resolve([]) };

const templates = dataStreams.map((dataStream) =>
prepareTemplate({ pkg: installablePackage, dataStream })
);
const templates = dataStreams.map((dataStream) => {
const experimentalDataStreamFeature = experimentalDataStreamFeatures.find(
(datastreamFeature) =>
datastreamFeature.data_stream === getRegistryDataStreamAssetBaseName(dataStream)
);

return prepareTemplate({ pkg: installablePackage, dataStream, experimentalDataStreamFeature });
});

const assetsToAdd = getAllTemplateRefs(templates.map((template) => template.indexTemplate));

return {
Expand Down Expand Up @@ -243,6 +252,7 @@ export function buildComponentTemplates(params: {
packageName: string;
pipelineName?: string;
defaultSettings: IndexTemplate['template']['settings'];
experimentalDataStreamFeature?: ExperimentalDataStreamFeature;
}) {
const {
templateName,
Expand Down Expand Up @@ -271,6 +281,10 @@ export function buildComponentTemplates(params: {
(dynampingTemplate) => Object.keys(dynampingTemplate)[0]
);

const sourceModeSynthetic =
params.experimentalDataStreamFeature?.features.synthetic_source !== false &&
(params.experimentalDataStreamFeature?.features.synthetic_source === true ||
registryElasticsearch?.source_mode === 'synthetic');
templatesMap[packageTemplateName] = {
template: {
settings: {
Expand All @@ -291,13 +305,11 @@ export function buildComponentTemplates(params: {
properties: mappingsProperties,
dynamic_templates: mappingsDynamicTemplates.length ? mappingsDynamicTemplates : undefined,
...omit(indexTemplateMappings, 'properties', 'dynamic_templates', '_source'),
...(indexTemplateMappings?._source || registryElasticsearch?.source_mode
...(indexTemplateMappings?._source || sourceModeSynthetic
? {
_source: {
...indexTemplateMappings?._source,
...(registryElasticsearch?.source_mode === 'synthetic'
? { mode: 'synthetic' }
: {}),
...(sourceModeSynthetic ? { mode: 'synthetic' } : {}),
},
}
: {}),
Expand Down Expand Up @@ -465,9 +477,11 @@ export async function ensureAliasHasWriteIndex(opts: {
export function prepareTemplate({
pkg,
dataStream,
experimentalDataStreamFeature,
}: {
pkg: Pick<PackageInfo, 'name' | 'version' | 'type'>;
dataStream: RegistryDataStream;
experimentalDataStreamFeature?: ExperimentalDataStreamFeature;
}): { componentTemplates: TemplateMap; indexTemplate: IndexTemplateEntry } {
const { name: packageName, version: packageVersion } = pkg;
const fields = loadFieldsFromYaml(pkg, dataStream.path);
Expand All @@ -494,6 +508,7 @@ export function prepareTemplate({
templateName,
pipelineName,
registryElasticsearch: dataStream.elasticsearch,
experimentalDataStreamFeature,
});

const template = getTemplate({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,16 @@ export async function _installPackage({
* This split of prepare/install could be extended to all asset types. Besides performance, it also allows us to
* more easily write unit tests against the asset generation code without needing to mock ES responses.
*/
const experimentalDataStreamFeatures =
installedPkg?.attributes?.experimental_data_stream_features ?? [];

const preparedIngestPipelines = prepareToInstallPipelines(packageInfo, paths);
const preparedIndexTemplates = prepareToInstallTemplates(packageInfo, paths, esReferences);
const preparedIndexTemplates = prepareToInstallTemplates(
packageInfo,
paths,
esReferences,
experimentalDataStreamFeatures
);

// Update the references for the templates and ingest pipelines together. Need to be done togther to avoid race
// conditions on updating the installed_es field at the same time
Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/fleet/server/services/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
SAVED_OBJECT_TYPE,
{
...packagePolicy,
...(packagePolicy.package
? { package: omit(packagePolicy.package, 'experimental_data_stream_features') }
: {}),
inputs,
elasticsearch,
revision: 1,
Expand Down Expand Up @@ -285,6 +288,9 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
id: packagePolicyId,
attributes: {
...pkgPolicyWithoutId,
...(packagePolicy.package
? { package: omit(packagePolicy.package, 'experimental_data_stream_features') }
: {}),
inputs,
elasticsearch,
policy_id: agentPolicyId,
Expand Down Expand Up @@ -526,6 +532,9 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
id,
{
...restOfPackagePolicy,
...(restOfPackagePolicy.package
? { package: omit(restOfPackagePolicy.package, 'experimental_data_stream_features') }
: {}),
inputs,
elasticsearch,
revision: oldPackagePolicy.revision + 1,
Expand Down Expand Up @@ -620,6 +629,9 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
id,
attributes: {
...restOfPackagePolicy,
...(restOfPackagePolicy.package
? { package: omit(restOfPackagePolicy.package, 'experimental_data_stream_features') }
: {}),
inputs,
elasticsearch,
revision: oldPackagePolicy.revision + 1,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/server/types/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export type {
PackageVerificationStatus,
BulkInstallPackageInfo,
PackageAssetReference,
ExperimentalDataStreamFeature,
} from '../../common/types';
export { ElasticsearchAssetType, KibanaAssetType, KibanaSavedObjectType } from '../../common/types';
export { dataTypes } from '../../common/constants';
Expand Down

0 comments on commit 4dc3f94

Please sign in to comment.