[Fleet][ON week] Remove extra manifest cache (#146987)

## Summary When looking into archive parsing performance i noticed we have another "secret" manifest buffer cache. Removing this doesn't affect performance and reduces kibana's memory usage after running `node scripts/get_all_packages` by approx 40mb. bonus: make get_all_packages script error more informative Co-authored-by: Kibana Machine <[email protected]>
elastic · Dec 5, 2022 · 6ee9cbf · 6ee9cbf
1 parent b0bd289
commit 6ee9cbf
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 37 deletions.
diff --git a/x-pack/plugins/fleet/scripts/get_all_packages/get_all_packages.ts b/x-pack/plugins/fleet/scripts/get_all_packages/get_all_packages.ts
@@ -35,28 +35,31 @@ interface Result {
 }
 async function getPackage(name: string, version: string, full: boolean = false) {
   const start = Date.now();
-  const res = await fetch(
-    `${KIBANA_URL}${base}/api/fleet/epm/packages/${name}/${version}?prerelease=true${
-      full ? '&full=true' : ''
-    }`,
-    {
-      headers: {
-        accept: '*/*',
-        'content-type': 'application/json',
-        'kbn-xsrf': 'xyz',
-        Authorization:
-          'Basic ' + Buffer.from(`${KIBANA_USERNAME}:${KIBANA_PASSWORD}`).toString('base64'),
-      },
-      method: 'GET',
-    }
-  );
-  const end = Date.now();
-
   let body;
+  let end;
+  let res;
   try {
+    res = await fetch(
+      `${KIBANA_URL}${base}/api/fleet/epm/packages/${name}/${version}?prerelease=true${
+        full ? '&full=true' : ''
+      }`,
+      {
+        headers: {
+          accept: '*/*',
+          'content-type': 'application/json',
+          'kbn-xsrf': 'xyz',
+          Authorization:
+            'Basic ' + Buffer.from(`${KIBANA_USERNAME}:${KIBANA_PASSWORD}`).toString('base64'),
+        },
+        method: 'GET',
+      }
+    );
+    end = Date.now();
+
     body = await res.json();
   } catch (e) {
-    logger.error(`Error parsing response: ${e}`);
+    logger.error(`Error reaching Kibana: ${e}`);
+    logger.info('If your kibana uses a base path, please set it with the --base /<your-base> flag');
     throw e;
   }
 

diff --git a/x-pack/plugins/fleet/server/services/epm/archive/parse.ts b/x-pack/plugins/fleet/server/services/epm/archive/parse.ts
@@ -38,7 +38,6 @@ import { pkgToPkgKey } from '../registry';
 import { unpackBufferEntries } from '.';
 
 const readFileAsync = promisify(readFile);
-const MANIFESTS: Record<string, Buffer> = {};
 const MANIFEST_NAME = 'manifest.yml';
 
 const DEFAULT_RELEASE_VALUE = 'ga';
@@ -80,6 +79,8 @@ export const expandDottedEntries = (obj: object) => {
   }, {} as Record<string, any>);
 };
 
+type ManifestMap = Record<string, Buffer>;
+
 // not sure these are 100% correct but they do the job here
 // keeping them local until others need them
 type OptionalPropertyOf<T extends object> = Exclude<
@@ -140,15 +141,16 @@ export async function generatePackageInfoFromArchiveBuffer(
   archiveBuffer: Buffer,
   contentType: string
 ): Promise<{ paths: string[]; packageInfo: ArchivePackage }> {
+  const manifests: ManifestMap = {};
   const entries = await unpackBufferEntries(archiveBuffer, contentType);
   const paths: string[] = [];
   entries.forEach(({ path: bufferPath, buffer }) => {
     paths.push(bufferPath);
-    if (bufferPath.endsWith(MANIFEST_NAME) && buffer) MANIFESTS[bufferPath] = buffer;
+    if (bufferPath.endsWith(MANIFEST_NAME) && buffer) manifests[bufferPath] = buffer;
   });
 
   return {
-    packageInfo: parseAndVerifyArchive(paths),
+    packageInfo: parseAndVerifyArchive(paths, manifests),
     paths,
   };
 }
@@ -161,15 +163,20 @@ export async function _generatePackageInfoFromPaths(
   paths: string[],
   topLevelDir: string
 ): Promise<ArchivePackage> {
+  const manifests: ManifestMap = {};
   await Promise.all(
     paths.map(async (filePath) => {
-      if (filePath.endsWith(MANIFEST_NAME)) MANIFESTS[filePath] = await readFileAsync(filePath);
+      if (filePath.endsWith(MANIFEST_NAME)) manifests[filePath] = await readFileAsync(filePath);
     })
   );
-  return parseAndVerifyArchive(paths, topLevelDir);
+  return parseAndVerifyArchive(paths, manifests, topLevelDir);
 }
 
-function parseAndVerifyArchive(paths: string[], topLevelDirOverride?: string): ArchivePackage {
+function parseAndVerifyArchive(
+  paths: string[],
+  manifests: ManifestMap,
+  topLevelDirOverride?: string
+): ArchivePackage {
   // The top-level directory must match pkgName-pkgVersion, and no other top-level files or directories may be present
   const toplevelDir = topLevelDirOverride || paths[0].split('/')[0];
   paths.forEach((filePath) => {
@@ -180,7 +187,7 @@ function parseAndVerifyArchive(paths: string[], topLevelDirOverride?: string): A
 
   // The package must contain a manifest file ...
   const manifestFile = path.posix.join(toplevelDir, MANIFEST_NAME);
-  const manifestBuffer = MANIFESTS[manifestFile];
+  const manifestBuffer = manifests[manifestFile];
   if (!paths.includes(manifestFile) || !manifestBuffer) {
     throw new PackageInvalidArchiveError(`Package must contain a top-level ${MANIFEST_NAME} file.`);
   }
@@ -217,12 +224,13 @@ function parseAndVerifyArchive(paths: string[], topLevelDirOverride?: string): A
     );
   }
 
-  const parsedDataStreams = parseAndVerifyDataStreams(
+  const parsedDataStreams = parseAndVerifyDataStreams({
     paths,
-    parsed.name,
-    parsed.version,
-    topLevelDirOverride
-  );
+    pkgName: parsed.name,
+    pkgVersion: parsed.version,
+    pkgBasePathOverride: topLevelDirOverride,
+    manifests,
+  });
 
   if (parsedDataStreams.length) {
     parsed.data_streams = parsedDataStreams;
@@ -253,12 +261,14 @@ function parseAndVerifyReadme(paths: string[], pkgName: string, pkgVersion: stri
   return paths.includes(readmePath) ? `/package/${pkgName}/${pkgVersion}${readmeRelPath}` : null;
 }
 
-export function parseAndVerifyDataStreams(
-  paths: string[],
-  pkgName: string,
-  pkgVersion: string,
-  pkgBasePathOverride?: string
-): RegistryDataStream[] {
+export function parseAndVerifyDataStreams(opts: {
+  paths: string[];
+  pkgName: string;
+  pkgVersion: string;
+  manifests: ManifestMap;
+  pkgBasePathOverride?: string;
+}): RegistryDataStream[] {
+  const { paths, pkgName, pkgVersion, manifests, pkgBasePathOverride } = opts;
   // A data stream is made up of a subdirectory of name-version/data_stream/, containing a manifest.yml
   const dataStreamPaths = new Set<string>();
   const dataStreams: RegistryDataStream[] = [];
@@ -277,7 +287,7 @@ export function parseAndVerifyDataStreams(
   dataStreamPaths.forEach((dataStreamPath) => {
     const fullDataStreamPath = path.posix.join(dataStreamsBasePath, dataStreamPath);
     const manifestFile = path.posix.join(fullDataStreamPath, MANIFEST_NAME);
-    const manifestBuffer = MANIFESTS[manifestFile];
+    const manifestBuffer = manifests[manifestFile];
     if (!paths.includes(manifestFile) || !manifestBuffer) {
       throw new PackageInvalidArchiveError(
         `No manifest.yml file found for data stream '${dataStreamPath}'`