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.

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

[Fleet] Fix datastream settings on package upgrade and package policy creation #147368

Merged
merged 14 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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