From 39954f572f82f6ac3a758dd67bea162a6ec4a227 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 8 Dec 2023 14:00:02 -0500 Subject: [PATCH] [8.12] [Fleet] Cache call to getBundledPackages (#172640) (#172978) # Backport This will backport the following commits from `main` to `8.12`: - [[Fleet] Cache call to getBundledPackages (#172640)](https://github.com/elastic/kibana/pull/172640) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) Co-authored-by: Nicolas Chaulet --- .../routes/fleet/get_latest_apm_package.ts | 2 +- x-pack/plugins/fleet/common/types/index.ts | 1 + .../plugins/fleet/common/types/models/epm.ts | 2 +- x-pack/plugins/fleet/server/config.ts | 3 + .../services/epm/package_service.test.ts | 8 +- .../server/services/epm/package_service.ts | 4 +- ...ckage.test.ts => bundled_packages.test.ts} | 89 ++++++++++++++----- .../services/epm/packages/bundled_packages.ts | 33 ++++++- .../services/epm/packages/install.test.ts | 2 +- .../server/services/epm/packages/install.ts | 4 +- .../services/epm/registry/index.test.ts | 2 +- .../server/services/epm/registry/index.ts | 3 +- .../server/services/preconfiguration.test.ts | 8 +- .../test/fleet_api_integration/config.base.ts | 1 + 14 files changed, 123 insertions(+), 39 deletions(-) rename x-pack/plugins/fleet/server/services/epm/packages/{bundled_package.test.ts => bundled_packages.test.ts} (50%) diff --git a/x-pack/plugins/apm/server/routes/fleet/get_latest_apm_package.ts b/x-pack/plugins/apm/server/routes/fleet/get_latest_apm_package.ts index 43528c037222e..efa6c1ee2d593 100644 --- a/x-pack/plugins/apm/server/routes/fleet/get_latest_apm_package.ts +++ b/x-pack/plugins/apm/server/routes/fleet/get_latest_apm_package.ts @@ -21,7 +21,7 @@ export async function getLatestApmPackage({ APM_PACKAGE_NAME ); const packageInfo = - 'buffer' in latestPackage + 'getBuffer' in latestPackage ? (await packageClient.readBundledPackage(latestPackage)).packageInfo : latestPackage; const { diff --git a/x-pack/plugins/fleet/common/types/index.ts b/x-pack/plugins/fleet/common/types/index.ts index 2deac11481473..dfee7502c4ed1 100644 --- a/x-pack/plugins/fleet/common/types/index.ts +++ b/x-pack/plugins/fleet/common/types/index.ts @@ -45,6 +45,7 @@ export interface FleetConfigType { disableRegistryVersionCheck?: boolean; bundledPackageLocation?: string; testSecretsIndex?: string; + disableBundledPackagesCache?: boolean; }; internal?: { disableILMPolicies: boolean; diff --git a/x-pack/plugins/fleet/common/types/models/epm.ts b/x-pack/plugins/fleet/common/types/models/epm.ts index 24bde5b5f5f5a..826f608b65864 100644 --- a/x-pack/plugins/fleet/common/types/models/epm.ts +++ b/x-pack/plugins/fleet/common/types/models/epm.ts @@ -132,7 +132,7 @@ export type ArchivePackage = PackageSpecManifest & export interface BundledPackage { name: string; version: string; - buffer: Buffer; + getBuffer: () => Promise; } export type RegistryPackage = PackageSpecManifest & diff --git a/x-pack/plugins/fleet/server/config.ts b/x-pack/plugins/fleet/server/config.ts index 87212ab025c25..0a817e0ccbfed 100644 --- a/x-pack/plugins/fleet/server/config.ts +++ b/x-pack/plugins/fleet/server/config.ts @@ -157,6 +157,9 @@ export const config: PluginConfigDescriptor = { disableRegistryVersionCheck: schema.boolean({ defaultValue: false }), allowAgentUpgradeSourceUri: schema.boolean({ defaultValue: false }), bundledPackageLocation: schema.string({ defaultValue: DEFAULT_BUNDLED_PACKAGE_LOCATION }), + disableBundledPackagesCache: schema.boolean({ + defaultValue: false, + }), }), packageVerification: schema.object({ gpgKeyPath: schema.string({ defaultValue: DEFAULT_GPG_KEY_PATH }), diff --git a/x-pack/plugins/fleet/server/services/epm/package_service.test.ts b/x-pack/plugins/fleet/server/services/epm/package_service.test.ts index ef9141ae9dfe5..9da46439a53c7 100644 --- a/x-pack/plugins/fleet/server/services/epm/package_service.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/package_service.test.ts @@ -169,12 +169,16 @@ function getTest( }; break; case testKeys[5]: - const bundledPackage = { name: 'package name', version: '8.0.0', buffer: Buffer.from([]) }; + const bundledPackage = { + name: 'package name', + version: '8.0.0', + getBuffer: () => Buffer.from([]), + }; test = { method: mocks.packageClient.readBundledPackage.bind(mocks.packageClient), args: [bundledPackage], spy: jest.spyOn(epmArchiveParse, 'generatePackageInfoFromArchiveBuffer'), - spyArgs: [bundledPackage.buffer, 'application/zip'], + spyArgs: [bundledPackage.getBuffer(), 'application/zip'], spyResponse: { packageInfo: { name: 'readBundledPackage test' }, paths: ['/some/test/path'], diff --git a/x-pack/plugins/fleet/server/services/epm/package_service.ts b/x-pack/plugins/fleet/server/services/epm/package_service.ts index e6f71cb7cb96c..eee1afb37dcaa 100644 --- a/x-pack/plugins/fleet/server/services/epm/package_service.ts +++ b/x-pack/plugins/fleet/server/services/epm/package_service.ts @@ -171,7 +171,9 @@ class PackageClientImpl implements PackageClient { public async readBundledPackage(bundledPackage: BundledPackage) { await this.#runPreflight(READ_PACKAGE_INFO_AUTHZ); - return generatePackageInfoFromArchiveBuffer(bundledPackage.buffer, 'application/zip'); + const archiveBuffer = await bundledPackage.getBuffer(); + + return generatePackageInfoFromArchiveBuffer(archiveBuffer, 'application/zip'); } public async getPackage( diff --git a/x-pack/plugins/fleet/server/services/epm/packages/bundled_package.test.ts b/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.test.ts similarity index 50% rename from x-pack/plugins/fleet/server/services/epm/packages/bundled_package.test.ts rename to x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.test.ts index f44a865e8992c..ff3ea5f0676c7 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/bundled_package.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.test.ts @@ -7,28 +7,37 @@ import fs from 'fs/promises'; +import { omit } from 'lodash'; + import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { appContextService } from '../../app_context'; -import { getBundledPackageByPkgKey, getBundledPackages } from './bundled_packages'; +import { + getBundledPackageByPkgKey, + getBundledPackages, + _purgeBundledPackagesCache, +} from './bundled_packages'; jest.mock('fs/promises'); jest.mock('../../app_context'); describe('bundledPackages', () => { - beforeAll(() => { + beforeEach(() => { jest.mocked(appContextService.getConfig).mockReturnValue({ developer: { bundledPackageLocation: '/tmp/test', }, } as any); jest.mocked(appContextService.getLogger).mockReturnValue(loggingSystemMock.createLogger()); - }); - beforeEach(() => { + _purgeBundledPackagesCache(); jest.mocked(fs.stat).mockResolvedValue({} as any); - jest.mocked(fs.readdir).mockResolvedValue(['apm-8.8.0.zip', 'test-1.0.0.zip'] as any); - jest.mocked(fs.readFile).mockResolvedValue(Buffer.from('TEST')); + jest + .mocked(fs.readdir) + .mockReset() + .mockResolvedValue(['apm-8.8.0.zip', 'test-1.0.0.zip'] as any); + + jest.mocked(fs.readFile).mockReset().mockResolvedValue(Buffer.from('TEST')); }); afterEach(() => { @@ -44,17 +53,47 @@ describe('bundledPackages', () => { it('return packages in bundled directory', async () => { const packages = await getBundledPackages(); expect(packages).toEqual([ - { + expect.objectContaining({ name: 'apm', version: '8.8.0', - buffer: Buffer.from('TEST'), - }, - { + }), + expect.objectContaining({ name: 'test', version: '1.0.0', - buffer: Buffer.from('TEST'), - }, + }), ]); + + expect(await packages[0]?.getBuffer()).toEqual(Buffer.from('TEST')); + expect(await packages[1]?.getBuffer()).toEqual(Buffer.from('TEST')); + }); + + it('should use cache if called multiple time', async () => { + const packagesRes1 = await getBundledPackages(); + const packagesRes2 = await getBundledPackages(); + expect(packagesRes1.map((p) => omit(p, 'getBuffer'))).toEqual( + packagesRes2.map((p) => omit(p, 'getBuffer')) + ); + expect(fs.readdir).toBeCalledTimes(1); + }); + + it('should cache getBuffer if called multiple time in the scope of getBundledPackages', async () => { + const packagesRes1 = await getBundledPackages(); + + await packagesRes1[0].getBuffer(); + await packagesRes1[0].getBuffer(); + expect(fs.readFile).toBeCalledTimes(1); + }); + + it('should not use cache if called multiple time and cache is disabled', async () => { + jest.mocked(appContextService.getConfig).mockReturnValue({ + developer: { + bundledPackageLocation: '/tmp/test', + disableBundledPackagesCache: true, + }, + } as any); + await getBundledPackages(); + await getBundledPackages(); + expect(fs.readdir).toBeCalledTimes(2); }); }); describe('getBundledPackageByPkgKey', () => { @@ -62,22 +101,28 @@ describe('bundledPackages', () => { const pkg = await getBundledPackageByPkgKey('apm'); expect(pkg).toBeDefined(); - expect(pkg).toEqual({ - name: 'apm', - version: '8.8.0', - buffer: Buffer.from('TEST'), - }); + expect(pkg).toEqual( + expect.objectContaining({ + name: 'apm', + version: '8.8.0', + }) + ); + + expect(await pkg?.getBuffer()).toEqual(Buffer.from('TEST')); }); it('should return package by name and version if version is provided', async () => { const pkg = await getBundledPackageByPkgKey('apm-8.8.0'); expect(pkg).toBeDefined(); - expect(pkg).toEqual({ - name: 'apm', - version: '8.8.0', - buffer: Buffer.from('TEST'), - }); + expect(pkg).toEqual( + expect.objectContaining({ + name: 'apm', + version: '8.8.0', + }) + ); + + expect(await pkg?.getBuffer()).toEqual(Buffer.from('TEST')); }); it('should return package by name and version if version is provided and do not exists', async () => { diff --git a/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts b/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts index 92f9674656103..9a6b09bc3e49e 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/bundled_packages.ts @@ -8,13 +8,36 @@ import fs from 'fs/promises'; import path from 'path'; +import { once } from 'lodash'; + import type { BundledPackage, Installation } from '../../../types'; import { BundledPackageLocationNotFoundError } from '../../../errors'; import { appContextService } from '../../app_context'; import { splitPkgKey, pkgToPkgKey } from '../registry'; +let CACHE_BUNDLED_PACKAGES: BundledPackage[] | undefined; + +export function _purgeBundledPackagesCache() { + CACHE_BUNDLED_PACKAGES = undefined; +} + +function bundledPackagesFromCache() { + if (!CACHE_BUNDLED_PACKAGES) { + throw new Error('CACHE_BUNDLED_PACKAGES is not populated'); + } + + return CACHE_BUNDLED_PACKAGES.map(({ name, version, getBuffer }) => ({ + name, + version, + getBuffer: once(getBuffer), + })); +} + export async function getBundledPackages(): Promise { const config = appContextService.getConfig(); + if (config?.developer?.disableBundledPackagesCache !== true && CACHE_BUNDLED_PACKAGES) { + return bundledPackagesFromCache(); + } const bundledPackageLocation = config?.developer?.bundledPackageLocation; @@ -38,19 +61,21 @@ export async function getBundledPackages(): Promise { const result = await Promise.all( zipFiles.map(async (zipFile) => { - const file = await fs.readFile(path.join(bundledPackageLocation, zipFile)); - const { pkgName, pkgVersion } = splitPkgKey(zipFile.replace(/\.zip$/, '')); + const getBuffer = () => fs.readFile(path.join(bundledPackageLocation, zipFile)); + return { name: pkgName, version: pkgVersion, - buffer: file, + getBuffer, }; }) ); - return result; + CACHE_BUNDLED_PACKAGES = result; + + return bundledPackagesFromCache(); } catch (err) { const logger = appContextService.getLogger(); logger.warn(`Unable to read bundled packages from ${bundledPackageLocation}`); diff --git a/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts b/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts index bc3e138b5619c..c58d14dd47320 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts @@ -269,7 +269,7 @@ describe('install', () => { mockGetBundledPackageByPkgKey.mockResolvedValue({ name: 'test_package', version: '1.0.0', - buffer: Buffer.from('test_package'), + getBuffer: async () => Buffer.from('test_package'), }); const response = await installPackage({ diff --git a/x-pack/plugins/fleet/server/services/epm/packages/install.ts b/x-pack/plugins/fleet/server/services/epm/packages/install.ts index 2dbf1662d4364..0760bd94a56ec 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.ts @@ -762,10 +762,12 @@ export async function installPackage(args: InstallPackageParams): Promise { mockGetBundledPackageByName.mockResolvedValueOnce({ name: 'test-package', version: '1.0.0', - buffer: Buffer.from(''), + getBuffer: async () => Buffer.from(''), }); MockArchive.generatePackageInfoFromArchiveBuffer.mockResolvedValueOnce({ paths: [], diff --git a/x-pack/plugins/fleet/server/services/epm/registry/index.ts b/x-pack/plugins/fleet/server/services/epm/registry/index.ts index 24c81dd023244..de513062f5873 100644 --- a/x-pack/plugins/fleet/server/services/epm/registry/index.ts +++ b/x-pack/plugins/fleet/server/services/epm/registry/index.ts @@ -198,8 +198,9 @@ export async function getBundledArchive( const bundledPackage = await getBundledPackageByName(pkgName); if (bundledPackage && bundledPackage.version === pkgVersion) { + const archiveBuffer = await bundledPackage.getBuffer(); const archivePackage = await generatePackageInfoFromArchiveBuffer( - bundledPackage.buffer, + archiveBuffer, 'application/zip' ); diff --git a/x-pack/plugins/fleet/server/services/preconfiguration.test.ts b/x-pack/plugins/fleet/server/services/preconfiguration.test.ts index fbeb2aa50c896..8c150aabb5b89 100644 --- a/x-pack/plugins/fleet/server/services/preconfiguration.test.ts +++ b/x-pack/plugins/fleet/server/services/preconfiguration.test.ts @@ -790,13 +790,13 @@ describe('policy preconfiguration', () => { { name: 'test_package', version: '1.0.0', - buffer: Buffer.from('test_package'), + getBuffer: () => Promise.resolve(Buffer.from('test_package')), }, { name: 'test_package_2', version: '1.0.0', - buffer: Buffer.from('test_package_2'), + getBuffer: () => Promise.resolve(Buffer.from('test_package_2')), }, ]); @@ -834,7 +834,7 @@ describe('policy preconfiguration', () => { { name: 'test_package', version: '1.0.0', - buffer: Buffer.from('test_package'), + getBuffer: () => Promise.resolve(Buffer.from('test_package')), }, ]); @@ -875,7 +875,7 @@ describe('policy preconfiguration', () => { { name: 'test_package', version: '1.0.0', - buffer: Buffer.from('test_package'), + getBuffer: () => Promise.resolve(Buffer.from('test_package')), }, ]); diff --git a/x-pack/test/fleet_api_integration/config.base.ts b/x-pack/test/fleet_api_integration/config.base.ts index 9649747d96632..5dfea767f98da 100644 --- a/x-pack/test/fleet_api_integration/config.base.ts +++ b/x-pack/test/fleet_api_integration/config.base.ts @@ -64,6 +64,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { `--xpack.fleet.packages.0.version=latest`, ...(registryPort ? [`--xpack.fleet.registryUrl=http://localhost:${registryPort}`] : []), `--xpack.fleet.developer.bundledPackageLocation=${BUNDLED_PACKAGE_DIR}`, + `--xpack.fleet.developer.disableBundledPackagesCache=true`, '--xpack.cloudSecurityPosture.enabled=true', `--xpack.fleet.developer.maxAgentPoliciesWithInactivityTimeout=10`, `--xpack.fleet.packageVerification.gpgKeyPath=${getFullPath(