Skip to content

Commit 17b8f14

Browse files
SupplementingSupplementing
authored
[Fleet] Fix missing reserved index pattern guard in streaming install path (#271749)
# Summary Closes elastic/ingest-dev#7887 `AutoInstallContentPackagesTask` (introduced in 9.2) could permanently delete user-created `metrics-*` and `logs-*` data views from all Kibana spaces on every content package upgrade cycle. The regular install path already filtered these reserved IDs via `removeReservedIndexPatterns` in `install.ts`, but the streaming path (`install_with_streaming.ts`) had no equivalent guard — so if a content package shipped a `metrics-*` index-pattern asset, the streaming path would record that ID in `installed_kibana`. On the next upgrade where the new version dropped the asset, `cleanUpUnusedKibanaAssetsStep` would delete the stale reference using an unscoped client, silently wiping the data view from every space. This PR adds two guards: (1) the streaming path now skips reserved index-pattern IDs before recording them as `installed_kibana refs`, and (2) `cleanUpUnusedKibanaAssetsStep` now explicitly excludes `logs-*` and `metrics-*` from cleanup as a defensive backstop for any IDs already incorrectly recorded in prior installs. ## Testing There is no easy way to trigger this manually without a content package that ships a metrics-* or logs-* index-pattern asset. The fix is covered by unit tests. To verify the guard is in place, the behavior to check after a streaming content package install is that {id: 'metrics-*', type: 'index-pattern'} and {id: 'logs-*', type: 'index-pattern'} do not appear in the package's installed_kibana attribute in the epm-packages saved object. ## Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation ](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)was added for features that require explanation or tutorials - [x] [Unit or functional tests ](https://www.elastic.co/guide/en/kibana/master/development-tests.html)were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) [ list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations. - [ ] [Flaky Test Runner ](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [ ] Review the [backport guidelines ](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)and apply applicable backport:* labels. ## Identify risks Low. The streaming path change only skips assets whose ID exactly matches logs-* or metrics-* — no other assets are affected. The cleanUpUnusedKibanaAssetsStep change extends an existing filtering pattern (already used for alert and alertingRuleTemplate) to cover these two IDs. Non-reserved index patterns continue to be installed and cleaned up normally.
1 parent 7889214 commit 17b8f14

4 files changed

Lines changed: 240 additions & 2 deletions

File tree

‎x-pack/platform/plugins/shared/fleet/server/services/epm/kibana/assets/install_with_streaming.test.ts‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import { savedObjectsClientMock } from '@kbn/core/server/mocks';
9+
10+
import { KibanaSavedObjectType } from '../../../../types';
11+
import { createArchiveIteratorFromMap } from '../../archive/archive_iterator';
12+
import { appContextService } from '../../../app_context';
13+
import { createAppContextStartContractMock } from '../../../../mocks';
14+
15+
import { installKibanaAssetsWithStreaming } from './install_with_streaming';
16+
17+
jest.mock('./saved_objects', () => ({
18+
getSpaceAwareSaveobjectsClients: jest.fn(),
19+
}));
20+
21+
jest.mock('./install', () => ({
22+
...jest.requireActual('./install'),
23+
installManagedIndexPattern: jest.fn().mockResolvedValue(undefined),
24+
}));
25+
26+
jest.mock('../../packages/install', () => ({
27+
...jest.requireActual('../../packages/install'),
28+
saveKibanaAssetsRefs: jest.fn().mockResolvedValue(undefined),
29+
}));
30+
31+
const { getSpaceAwareSaveobjectsClients } = jest.requireMock('./saved_objects');
32+
33+
const makeArchiveBuffer = (id: string, soType: string) =>
34+
Buffer.from(JSON.stringify({ id, type: soType, attributes: { title: id } }));
35+
36+
describe('installKibanaAssetsWithStreaming', () => {
37+
let soClient: ReturnType<typeof savedObjectsClientMock.create>;
38+
let soClientWithSpace: ReturnType<typeof savedObjectsClientMock.create>;
39+
40+
beforeEach(() => {
41+
soClient = savedObjectsClientMock.create();
42+
soClientWithSpace = savedObjectsClientMock.create();
43+
soClientWithSpace.bulkCreate.mockResolvedValue({ saved_objects: [] });
44+
45+
getSpaceAwareSaveobjectsClients.mockReturnValue({
46+
savedObjectClientWithSpace: soClientWithSpace,
47+
savedObjectsImporter: { import: jest.fn().mockResolvedValue({ errors: [] }) },
48+
savedObjectTagAssignmentService: jest.fn(),
49+
savedObjectTagClient: jest.fn(),
50+
});
51+
52+
appContextService.start(createAppContextStartContractMock());
53+
});
54+
55+
it('should not include reserved index patterns (metrics-*, logs-*) in returned assetRefs', async () => {
56+
const assetsMap = new Map([
57+
[
58+
'test-package-1.0.0/kibana/index_pattern/metrics-star.json',
59+
makeArchiveBuffer('metrics-*', 'index-pattern'),
60+
],
61+
[
62+
'test-package-1.0.0/kibana/index_pattern/logs-star.json',
63+
makeArchiveBuffer('logs-*', 'index-pattern'),
64+
],
65+
[
66+
'test-package-1.0.0/kibana/dashboard/my-dashboard.json',
67+
makeArchiveBuffer('my-dashboard', 'dashboard'),
68+
],
69+
]);
70+
71+
const refs = await installKibanaAssetsWithStreaming({
72+
spaceId: 'default',
73+
packageInstallContext: {
74+
archiveIterator: createArchiveIteratorFromMap(assetsMap),
75+
paths: [...assetsMap.keys()],
76+
packageInfo: {
77+
title: 'Test',
78+
name: 'test-package',
79+
version: '1.0.0',
80+
description: 'test',
81+
type: 'integration',
82+
categories: [],
83+
format_version: '1.0.0',
84+
release: 'ga',
85+
conditions: {},
86+
owner: { github: 'elastic/fleet' },
87+
} as any,
88+
},
89+
savedObjectsClient: soClient,
90+
pkgName: 'test-package',
91+
});
92+
93+
const refIds = refs.map((r) => r.id);
94+
expect(refIds).not.toContain('metrics-*');
95+
expect(refIds).not.toContain('logs-*');
96+
expect(refIds).toContain('my-dashboard');
97+
});
98+
99+
it('should return empty refs when archive contains only reserved index patterns', async () => {
100+
const assetsMap = new Map([
101+
[
102+
'test-package-1.0.0/kibana/index_pattern/metrics-star.json',
103+
makeArchiveBuffer('metrics-*', 'index-pattern'),
104+
],
105+
[
106+
'test-package-1.0.0/kibana/index_pattern/logs-star.json',
107+
makeArchiveBuffer('logs-*', 'index-pattern'),
108+
],
109+
]);
110+
111+
const refs = await installKibanaAssetsWithStreaming({
112+
spaceId: 'default',
113+
packageInstallContext: {
114+
archiveIterator: createArchiveIteratorFromMap(assetsMap),
115+
paths: [...assetsMap.keys()],
116+
packageInfo: {
117+
title: 'Test',
118+
name: 'test-package',
119+
version: '1.0.0',
120+
description: 'test',
121+
type: 'integration',
122+
categories: [],
123+
format_version: '1.0.0',
124+
release: 'ga',
125+
conditions: {},
126+
owner: { github: 'elastic/fleet' },
127+
} as any,
128+
},
129+
savedObjectsClient: soClient,
130+
pkgName: 'test-package',
131+
});
132+
133+
expect(refs).toEqual([]);
134+
expect(soClientWithSpace.bulkCreate).not.toBeCalled();
135+
});
136+
137+
it('should include non-reserved index patterns in returned assetRefs', async () => {
138+
const assetsMap = new Map([
139+
[
140+
'test-package-1.0.0/kibana/index_pattern/custom-index-pattern.json',
141+
makeArchiveBuffer('custom-index-pattern', 'index-pattern'),
142+
],
143+
]);
144+
145+
const refs = await installKibanaAssetsWithStreaming({
146+
spaceId: 'default',
147+
packageInstallContext: {
148+
archiveIterator: createArchiveIteratorFromMap(assetsMap),
149+
paths: [...assetsMap.keys()],
150+
packageInfo: {
151+
title: 'Test',
152+
name: 'test-package',
153+
version: '1.0.0',
154+
description: 'test',
155+
type: 'integration',
156+
categories: [],
157+
format_version: '1.0.0',
158+
release: 'ga',
159+
conditions: {},
160+
owner: { github: 'elastic/fleet' },
161+
} as any,
162+
},
163+
savedObjectsClient: soClient,
164+
pkgName: 'test-package',
165+
});
166+
167+
expect(refs).toEqual([
168+
{ id: 'custom-index-pattern', type: KibanaSavedObjectType.indexPattern },
169+
]);
170+
});
171+
});

‎x-pack/platform/plugins/shared/fleet/server/services/epm/kibana/assets/install_with_streaming.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import { appContextService } from '../../../app_context';
1919

2020
import { saveKibanaAssetsRefs } from '../../packages/install';
2121

22+
import { indexPatternTypes } from '../index_pattern/install';
23+
2224
import type { ArchiveAsset } from './install';
2325
import {
2426
KibanaSavedObjectTypeMapping,
@@ -83,6 +85,13 @@ export async function installKibanaAssetsWithStreaming({
8385
return;
8486
}
8587

88+
if (
89+
soType === KibanaSavedObjectType.indexPattern &&
90+
indexPatternTypes.some((pattern) => `${pattern}-*` === savedObject.id)
91+
) {
92+
return;
93+
}
94+
8695
batch.push(savedObject);
8796
assetRefs.push(toAssetReference(savedObject));
8897

‎x-pack/platform/plugins/shared/fleet/server/services/epm/packages/install_state_machine/steps/step_install_kibana_assets.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,10 @@ describe('cleanUpUnusedKibanaAssetsStep', () => {
412412
appContextService.start(createAppContextStartContractMock());
413413
});
414414

415+
afterEach(() => {
416+
mockedDeleteKibanaAssets.mockReset();
417+
});
418+
415419
it('should not clean up assets if they all present in the new package', async () => {
416420
const installedAssets = [{ type: KibanaSavedObjectType.dashboard, id: 'dashboard-1' }];
417421
await cleanUpUnusedKibanaAssetsStep({
@@ -454,4 +458,51 @@ describe('cleanUpUnusedKibanaAssetsStep', () => {
454458
logger: expect.anything(),
455459
});
456460
});
461+
462+
it('should never delete reserved Fleet index patterns (metrics-*, logs-*) even if absent from new package', async () => {
463+
const previousAssets = [
464+
{ type: KibanaSavedObjectType.dashboard, id: 'dashboard-to-remove' },
465+
{ type: KibanaSavedObjectType.indexPattern, id: 'metrics-*' },
466+
{ type: KibanaSavedObjectType.indexPattern, id: 'logs-*' },
467+
];
468+
469+
await cleanUpUnusedKibanaAssetsStep({
470+
...installationContext,
471+
installedPkg: {
472+
...mockInstalledPackageSo,
473+
attributes: {
474+
...mockInstalledPackageSo.attributes,
475+
installed_kibana: previousAssets,
476+
},
477+
},
478+
installedKibanaAssetsRefs: [],
479+
});
480+
481+
expect(mockedDeleteKibanaAssets).toBeCalledWith(
482+
expect.objectContaining({
483+
installedObjects: [{ type: KibanaSavedObjectType.dashboard, id: 'dashboard-to-remove' }],
484+
})
485+
);
486+
});
487+
488+
it('should not call deleteKibanaAssets at all when only reserved index patterns would be removed', async () => {
489+
const previousAssets = [
490+
{ type: KibanaSavedObjectType.indexPattern, id: 'metrics-*' },
491+
{ type: KibanaSavedObjectType.indexPattern, id: 'logs-*' },
492+
];
493+
494+
await cleanUpUnusedKibanaAssetsStep({
495+
...installationContext,
496+
installedPkg: {
497+
...mockInstalledPackageSo,
498+
attributes: {
499+
...mockInstalledPackageSo.attributes,
500+
installed_kibana: previousAssets,
501+
},
502+
},
503+
installedKibanaAssetsRefs: [],
504+
});
505+
506+
expect(mockedDeleteKibanaAssets).not.toBeCalled();
507+
});
457508
});

‎x-pack/platform/plugins/shared/fleet/server/services/epm/packages/install_state_machine/steps/step_install_kibana_assets.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { deleteKibanaAssets } from '../../remove';
1414
import type { KibanaAssetReference } from '../../../../../../common/types';
1515
import { INSTALL_STATES, KibanaSavedObjectType } from '../../../../../../common/types';
1616
import { installKibanaAssetsWithStreaming } from '../../../kibana/assets/install_with_streaming';
17+
import { indexPatternTypes } from '../../../kibana/index_pattern/install';
1718

1819
export async function stepInstallKibanaAssets(context: InstallContext) {
1920
const { savedObjectsClient, logger, installedPkg, packageInstallContext, spaceId } = context;
@@ -125,12 +126,18 @@ export async function cleanUpUnusedKibanaAssetsStep(context: InstallContext) {
125126
const nextAssetRefKeys = new Set(
126127
installedKibanaAssetsRefs.map((asset: KibanaAssetReference) => `${asset.id}-${asset.type}`)
127128
);
128-
// Do not remove alerting rules or alerting rule templates (they are managed separately)
129+
const reservedIndexPatternIds = new Set(indexPatternTypes.map((pattern) => `${pattern}-*`));
130+
131+
// Do not remove alerting rules, alerting rule templates, or reserved Fleet index patterns
129132
const assetsToRemove = previousAssetRefs.filter(
130133
(existingAsset) =>
131134
!nextAssetRefKeys.has(`${existingAsset.id}-${existingAsset.type}`) &&
132135
existingAsset.type !== KibanaSavedObjectType.alert &&
133-
existingAsset.type !== KibanaSavedObjectType.alertingRuleTemplate
136+
existingAsset.type !== KibanaSavedObjectType.alertingRuleTemplate &&
137+
!(
138+
existingAsset.type === KibanaSavedObjectType.indexPattern &&
139+
reservedIndexPatternIds.has(existingAsset.id)
140+
)
134141
);
135142

136143
if (assetsToRemove.length === 0) {

0 commit comments

Comments
 (0)